Refactor AudioInputDevice to remove race conditions and allow more flexible calling sequences.

content/renderer/media/audio_input_device.cc

Index: content/renderer/media/audio_input_device.cc
===================================================================
--- content/renderer/media/audio_input_device.cc	(revision 96746)
+++ content/renderer/media/audio_input_device.cc	(working copy)
@@ -32,19 +32,12 @@
 }
 
 AudioInputDevice::~AudioInputDevice() {
-  // Make sure we have been shut down.
-  DCHECK_EQ(0, stream_id_);
   Stop();
   for (int i = 0; i < channels_; ++i)
     delete [] audio_data_[i];
 }
 
-bool AudioInputDevice::Start() {
-  // Make sure we don't call Start() more than once.
-  DCHECK_EQ(0, stream_id_);
-  if (stream_id_)
-    return false;
-
+void AudioInputDevice::Start() {
   AudioParameters params;
   // TODO(henrika): add support for low-latency mode?
   params.format = AudioParameters::AUDIO_PCM_LINEAR;
@@ -56,21 +49,31 @@
   ChildProcess::current()->io_message_loop()->PostTask(
     FROM_HERE,
     NewRunnableMethod(this, &AudioInputDevice::InitializeOnIOThread, params));
-
-  return true;
 }
 
 bool AudioInputDevice::Stop() {
-  if (!stream_id_)
-    return false;
+  base::WaitableEvent completion(false, false);
 
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
-      NewRunnableMethod(this, &AudioInputDevice::ShutDownOnIOThread));
+      NewRunnableMethod(this, &AudioInputDevice::ShutDownOnIOThread,
+                        &completion));
 
-  if (audio_thread_.get()) {
-    socket_->Close();
-    audio_thread_->Join();
+  // We wait here for the IO task to be completed to remove race conflicts
+  // with OnLowLatencyCreated and to ensure that Stop can be called from
+  // the destructor.
+  if (completion.Wait()) {
+    if (audio_thread_.get()) {
+      // Terminate the main thread function in the audio thread.
+      socket_->Close();
+      // Wait for the audio thread to exit.
+      audio_thread_->Join();
+      // Ensures that we can call Stop multiple times.
+      audio_thread_.reset(NULL);
+    }
+  } else {
+    DLOG(ERROR) << "failed to shut down audio input on IO thread";
+    return false;
   }
 
   return true;
@@ -87,6 +90,11 @@
 }
 
 void AudioInputDevice::InitializeOnIOThread(const AudioParameters& params) {

[tommi (sloooow) - chröme, 2011/08/17 08:35:18]

DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop())

[henrika (OOO until Aug 14), 2011/08/17 15:34:13]

Done.

+  // Make sure we don't call Start() more than once.
+  DCHECK_EQ(0, stream_id_);
+  if (stream_id_)
+    return;
+
   stream_id_ = filter_->AddDelegate(this);
   Send(new AudioInputHostMsg_CreateStream(stream_id_, params, true));
 }
@@ -96,14 +104,18 @@
     Send(new AudioInputHostMsg_RecordStream(stream_id_));
 }
 
-void AudioInputDevice::ShutDownOnIOThread() {
+void AudioInputDevice::ShutDownOnIOThread(base::WaitableEvent* completion) {
   // Make sure we don't call shutdown more than once.

[tommi (sloooow) - chröme, 2011/08/17 08:35:18]

DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop())

[henrika (OOO until Aug 14), 2011/08/17 15:34:13]

Done.

-  if (!stream_id_)
+  if (!stream_id_) {
+    completion->Signal();
     return;
+  }
 
   filter_->RemoveDelegate(stream_id_);
   Send(new AudioInputHostMsg_CloseStream(stream_id_));
   stream_id_ = 0;
+
+  completion->Signal();
 }
 
 void AudioInputDevice::SetVolumeOnIOThread(double volume) {
@@ -125,13 +137,20 @@
 #endif
   DCHECK(length);
 
+  // Takes care of the case when Stop is called before OnLowLatencyCreated.
+  if (!stream_id_) {
+    base::SharedMemory::CloseHandle(handle);
+    base::SyncSocket socket(socket_handle);

[tommi (sloooow) - chröme, 2011/08/17 08:35:18]

is this the normal way of closing a socket?  i.e. is there no static function?

[henrika (OOO until Aug 14), 2011/08/17 15:34:13]

Not sure if it is normal but it works ;-) And, it seems to be used on other
places in Chrome as well. Please note that Close() is called during destruction.

+    return;
+  }
+
   shared_memory_.reset(new base::SharedMemory(handle, false));
   shared_memory_->Map(length);
 
   socket_.reset(new base::SyncSocket(socket_handle));
 
   audio_thread_.reset(
-      new base::DelegateSimpleThread(this, "renderer_audio_input_thread"));
+      new base::DelegateSimpleThread(this, "RendererAudioInputThread"));
   audio_thread_->Start();
 
   MessageLoop::current()->PostTask(