Prevent AudioDeviceThread from starting if clients have called Stop() (round 2).

media/audio/audio_output_device.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/audio_output_device.h"
 
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
@@ skipping 31 matching lines @@
 
 AudioOutputDevice::AudioOutputDevice(
     AudioOutputIPC* ipc,
     const scoped_refptr<base::MessageLoopProxy>& io_loop)
     : ScopedLoopObserver(io_loop),
       input_channels_(0),
       callback_(NULL),
       ipc_(ipc),
       stream_id_(0),
       play_on_start_(true),
-      is_started_(false) {
+      is_started_(false),
+      stopping_hack_(false) {
   CHECK(ipc_);
 }
 
 void AudioOutputDevice::Initialize(const AudioParameters& params,
                                    RenderCallback* callback) {
   CHECK_EQ(0, stream_id_) <<
       "AudioOutputDevice::Initialize() must be called before Start()";
 
   CHECK(!callback_);  // Calling Initialize() twice?
 
@@ skipping 20 matching lines @@
   DCHECK(callback_) << "Initialize hasn't been called";
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioOutputDevice::CreateStreamOnIOThread, this,
                  audio_parameters_, input_channels_));
 }
 
 void AudioOutputDevice::Stop() {
   {
     base::AutoLock auto_lock(audio_thread_lock_);
     audio_thread_.Stop(MessageLoop::current());
+    stopping_hack_ = true;
   }
 
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioOutputDevice::ShutDownOnIOThread, this));
 }
 
 void AudioOutputDevice::Play() {
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioOutputDevice::PlayOnIOThread, this));
 }
@@ skipping 60 matching lines @@
       ipc_->RemoveDelegate(stream_id_);
     }
 
     stream_id_ = 0;
   }
 
   // We can run into an issue where ShutDownOnIOThread is called right after
   // OnStreamCreated is called in cases where Start/Stop are called before we
   // get the OnStreamCreated callback.  To handle that corner case, we call
   // Stop(). In most cases, the thread will already be stopped.
+  //
   // Another situation is when the IO thread goes away before Stop() is called
   // in which case, we cannot use the message loop to close the thread handle
-  // and can't not rely on the main thread existing either.
+  // and can't rely on the main thread existing either.
+  base::AutoLock auto_lock_(audio_thread_lock_);
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   audio_thread_.Stop(NULL);
   audio_callback_.reset();
+  stopping_hack_ = false;
 }
 
 void AudioOutputDevice::SetVolumeOnIOThread(double volume) {
   DCHECK(message_loop()->BelongsToCurrentThread());
   if (stream_id_)
     ipc_->SetVolume(stream_id_, volume);
 }
 
 void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegate::State state) {
   DCHECK(message_loop()->BelongsToCurrentThread());
@@ skipping 28 matching lines @@
   DCHECK_GE(socket_handle, 0);
 #endif
 
   // We should only get this callback if stream_id_ is valid.  If it is not,
   // the IPC layer should have closed the shared memory and socket handles
   // for us and not invoked the callback.  The basic assertion is that when
   // stream_id_ is 0 the AudioOutputDevice instance is not registered as a
   // delegate and hence it should not receive callbacks.
   DCHECK(stream_id_);
 
+  // We can receive OnStreamCreated() on the IO thread after the client has
+  // called Stop() but before ShutDownOnIOThread() is processed. In such a
+  // situation |callback_| might point to freed memory. Instead of starting
+  // |audio_thread_| do nothing and wait for ShutDownOnIOThread() to get called.
+  //
+  // TODO(scherkus): The real fix is to have sane ownership semantics. The fact
+  // that |callback_| (which should own and outlive this object!) can point to
+  // freed memory is a mess. AudioRendererSink should be non-refcounted so that
+  // owners (WebRtcAudioDeviceImpl, AudioRendererImpl, etc...) can Stop() and
+  // delete as they see fit. AudioOutputDevice should internally use WeakPtr
+  // to handle teardown and thread hopping. See http://crbug.com/151051 for
+  // details.
   base::AutoLock auto_lock(audio_thread_lock_);
+  if (stopping_hack_)
+    return;
 
   DCHECK(audio_thread_.IsStopped());
   audio_callback_.reset(new AudioOutputDevice::AudioThreadCallback(
       audio_parameters_, input_channels_, handle, length, callback_));
   audio_thread_.Start(audio_callback_.get(), socket_handle,
       "AudioOutputDevice");
 
   // We handle the case where Play() and/or Pause() may have been called
   // multiple times before OnStreamCreated() gets called.
   is_started_ = true;
@@ skipping 85 matching lines @@
   // TODO(dalecurtis): Technically this is not always correct.  Due to channel
   // padding for alignment, there may be more data available than this.  We're
   // relying on AudioSyncReader::Read() to parse this with that in mind.  Rename
   // these methods to Set/GetActualFrameCount().
   SetActualDataSizeInBytes(
       &shared_memory_, memory_length_,
       num_frames * sizeof(*output_bus_->channel(0)) * output_bus_->channels());
 }
 
 }  // namespace media.