Stop the AudioDeviceThread when the IO loop goes away.

When shutting down, the pipeline code can take a long time to terminate.
In some rare cases, the IO loop terminates before the pipeline thread and
the audio device classes would attempt to use the io loop to perform tasks on.
I checked in a candidate fix for this a couple of weeks ago which monitors the
lifetime of the IO loop and uses MessageLoopProxy* instead of MessageLoop* to
avoid issues with using the NULL loop.  That fixes part of the problem.
What was missing was also stopping the audio thread in this case since the IO
thread will run teardown before the Pipeline thread calls AudioDevice::Stop().
In that period, the audio thread will still be running but the callback object
has been deleted.

Additionally, I'm adding a lock to guard the thread_ variable in AudioDeviceThread.
Before I was relying on external synchronization in the AudioDevice and
AudioInputDevice state machine.  However, it doesn't hurt to go belt and
suspenders although it might not look cool ;)

BUG=115299
TEST=Follow repro steps in bug report.


Review URL: http://codereview.chromium.org/9534002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124330 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/media/audio_device.cc

@@ -178,6 +178,15 @@ void AudioDevice::ShutDownOnIOThread() {
     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.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  audio_thread_.Stop(NULL);
   audio_callback_.reset();
 }
 
@@ -218,6 +227,7 @@ void AudioDevice::OnStreamCreated(
     return;
   }
 
+  DCHECK(audio_thread_.IsStopped());
   audio_callback_.reset(new AudioDevice::AudioThreadCallback(audio_parameters_,
       handle, length, callback_));
   audio_thread_.Start(audio_callback_.get(), socket_handle, "AudioDevice");
@@ -234,6 +244,7 @@ void AudioDevice::Send(IPC::Message* message) {
 }
 
 void AudioDevice::WillDestroyCurrentMessageLoop() {
+  LOG(ERROR) << "IO loop going away before the audio device has been stopped";
   ShutDownOnIOThread();
 }
 

content/renderer/media/audio_device_thread.cc

@@ -7,7 +7,6 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
-#include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "media/audio/audio_util.h"
@@ -27,6 +26,11 @@ class AudioDeviceThread::Thread
          const char* thread_name);
 
   void Start();
+
+  // Stops the thread.  If |loop_for_join| is non-NULL, the function posts
+  // a task to join (close) the thread handle later instead of waiting for
+  // the thread.  If loop_for_join is NULL, then the function waits
+  // synchronously for the thread to terminate.
   void Stop(MessageLoop* loop_for_join);
 
  private:
@@ -61,22 +65,25 @@ AudioDeviceThread::~AudioDeviceThread() {
 void AudioDeviceThread::Start(AudioDeviceThread::Callback* callback,
                               base::SyncSocket::Handle socket,
                               const char* thread_name) {
+  base::AutoLock auto_lock(thread_lock_);
   CHECK(thread_ == NULL);
   thread_ = new AudioDeviceThread::Thread(callback, socket, thread_name);
   thread_->Start();
 }
 
 void AudioDeviceThread::Stop(MessageLoop* loop_for_join) {
+  base::AutoLock auto_lock(thread_lock_);
   if (thread_) {
-    if (!loop_for_join) {
-      loop_for_join = MessageLoop::current();
-      CHECK(loop_for_join);
-    }
     thread_->Stop(loop_for_join);
     thread_ = NULL;
   }
 }
 
+bool AudioDeviceThread::IsStopped() {
+  base::AutoLock auto_lock(thread_lock_);
+  return thread_ == NULL;
+}
+
 // AudioDeviceThread::Thread implementation
 AudioDeviceThread::Thread::Thread(AudioDeviceThread::Callback* callback,
                                   base::SyncSocket::Handle socket,
@@ -113,8 +120,12 @@ void AudioDeviceThread::Thread::Stop(MessageLoop* loop_for_join) {
   }
 
   if (thread != base::kNullThreadHandle) {
-    loop_for_join->PostTask(FROM_HERE,
-        base::Bind(&base::PlatformThread::Join, thread));
+    if (loop_for_join) {
+      loop_for_join->PostTask(FROM_HERE,
+          base::Bind(&base::PlatformThread::Join, thread));
+    } else {
+      base::PlatformThread::Join(thread);
+    }
   }
 }
 

content/renderer/media/audio_device_thread.h

@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "base/sync_socket.h"
+#include "base/synchronization/lock.h"
 #include "content/common/content_export.h"
 #include "media/audio/audio_parameters.h"
 
@@ -76,16 +77,19 @@ class CONTENT_EXPORT AudioDeviceThread {
              const char* thread_name);
 
   // This tells the audio thread to stop and clean up the data.
-  // The method is asynchronous, so the thread might still be running after it
+  // The method can stop the thread synchronously or asynchronously.
+  // In the latter case, the thread will still be running after Stop()
   // returns, but the callback pointer is cleared so no further callbacks will
-  // be made (i.e. after Stop() returns, it is safe to delete the callback).
-  // The |loop_for_join| parameter is required in order to join the worker
-  // thread and close the thread handle later via a posted task.
-  // If set to NULL, the current message loop is used. Note that the thread
-  // that the message loop belongs to, must be allowed to join threads
-  // (see SetIOAllowed in base/thread_restrictions.h).
+  // be made (IOW after Stop() returns, it is safe to delete the callback).
+  // The |loop_for_join| parameter is required for asynchronous operation
+  // in order to join the worker thread and close the thread handle later via a
+  // posted task.
+  // If set to NULL, function will wait for the thread to exit before returning.
   void Stop(MessageLoop* loop_for_join);
 
+  // Returns true if the thread is stopped or stopping.
+  bool IsStopped();
+
  private:
   // Our own private SimpleThread override.  We implement this in a
   // private class so that we get the following benefits:
@@ -96,6 +100,7 @@ class CONTENT_EXPORT AudioDeviceThread {
   //    reliable initialization.
   class Thread;
 
+  base::Lock thread_lock_;
   scoped_refptr<AudioDeviceThread::Thread> thread_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioDeviceThread);

content/renderer/media/audio_input_device.cc

@@ -146,6 +146,16 @@ void AudioInputDevice::ShutDownOnIOThread() {
     session_id_ = 0;
     pending_device_ready_ = false;
   }
+
+  // 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.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  audio_thread_.Stop(NULL);
   audio_callback_.reset();
 }
 
@@ -178,11 +188,11 @@ void AudioInputDevice::OnStreamCreated(
     return;
   }
 
+  DCHECK(audio_thread_.IsStopped());
   audio_callback_.reset(
       new AudioInputDevice::AudioThreadCallback(audio_parameters_, handle,
                                                 length, callback_));
-  audio_thread_.Start(audio_callback_.get(), socket_handle,
-                       "AudioInputDevice");
+  audio_thread_.Start(audio_callback_.get(), socket_handle, "AudioInputDevice");
 
   MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&AudioInputDevice::StartOnIOThread, this));
@@ -258,6 +268,7 @@ void AudioInputDevice::Send(IPC::Message* message) {
 }
 
 void AudioInputDevice::WillDestroyCurrentMessageLoop() {
+  LOG(ERROR) << "IO loop going away before the input device has been stopped";
   ShutDownOnIOThread();
 }