Fix race in AudioInputController::Close().

Before this change, a reference to AudioInputController was bound into the
Callback invoking DoClose(), which executes on the audio thread.  If the
audio thread is slow to delete that callback (after dispatching |closed_task|)
then the main thread's reference to the AudioInputController might be released
first, leading to AudioInputController being destructed on the audio thread,
triggering a DCHECK in Timer::~Timer() (owned by the AudioInputController via
DelayTimer).

This race was exposed while working on http://codereview.chromium.org/9717021/
but isn't so much related to the content of that CL as to the fact that that CL
twiddles callback lifetime/timing a bit.


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

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

3 files changed, 13 insertions, 21 deletions

diff --git a/media/audio/audio_input_controller.cc b/media/audio/audio_input_controller.cc
index 27903fa..67623da 100644
--- a/media/audio/audio_input_controller.cc
+++ b/media/audio/audio_input_controller.cc
@@ -103,8 +103,8 @@ void AudioInputController::Record() {
 
 void AudioInputController::Close(const base::Closure& closed_task) {
   DCHECK(!closed_task.is_null());
-  message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioInputController::DoClose, this, closed_task));
+  message_loop_->PostTaskAndReply(
+      FROM_HERE, base::Bind(&AudioInputController::DoClose, this), closed_task);
 }
 
 void AudioInputController::DoCreate(AudioManager* audio_manager,
@@ -150,7 +150,7 @@ void AudioInputController::DoRecord() {
   handler_->OnRecording(this);
 }
 
-void AudioInputController::DoClose(const base::Closure& closed_task) {
+void AudioInputController::DoClose() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   if (state_ != kClosed) {
@@ -162,8 +162,6 @@ void AudioInputController::DoClose(const base::Closure& closed_task) {
 
     state_ = kClosed;
   }
-
-  closed_task.Run();
 }
 
 void AudioInputController::DoReportError(int code) {
diff --git a/media/audio/audio_input_controller.h b/media/audio/audio_input_controller.h
index 13815ee..59f8d16 100644
--- a/media/audio/audio_input_controller.h
+++ b/media/audio/audio_input_controller.h
@@ -148,12 +148,13 @@ class MEDIA_EXPORT AudioInputController
   virtual void Record();
 
   // Closes the audio input stream. The state is changed and the resources
-  // are freed on the audio thread. |closed_task| is executed after that.
+  // are freed on the audio thread. |closed_task| is then executed on the thread
+  // that called Close().
   // Callbacks (EventHandler and SyncWriter) must exist until |closed_task|
   // is called.
   // It is safe to call this method more than once. Calls after the first one
   // will have no effect.
-  // This method is called on the audio thread.
+  // This method trampolines to the audio thread.
   virtual void Close(const base::Closure& closed_task);
 
   // AudioInputCallback implementation. Threading details depends on the
@@ -184,7 +185,7 @@ class MEDIA_EXPORT AudioInputController
   void DoCreate(AudioManager* audio_manager, const AudioParameters& params,
                 const std::string& device_id);
   void DoRecord();
-  void DoClose(const base::Closure& closed_task);
+  void DoClose();
   void DoReportError(int code);
 
   // Methods which ensures that OnError() is triggered when data recording
diff --git a/media/audio/audio_input_controller_unittest.cc b/media/audio/audio_input_controller_unittest.cc
index 250f7c8..869610e 100644
--- a/media/audio/audio_input_controller_unittest.cc
+++ b/media/audio/audio_input_controller_unittest.cc
@@ -37,10 +37,8 @@ ACTION_P3(CheckCountAndPostQuitTask, count, limit, loop_or_proxy) {
 
 // Closes AudioOutputController synchronously.
 static void CloseAudioController(AudioInputController* controller) {
-  base::WaitableEvent closed_event(true, false);
-  controller->Close(base::Bind(&base::WaitableEvent::Signal,
-                               base::Unretained(&closed_event)));
-  closed_event.Wait();
+  controller->Close(MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
 }
 
 class MockAudioInputControllerEventHandler
@@ -216,16 +214,11 @@ TEST_F(AudioInputControllerTest, CloseTwice) {
 
   controller->Record();
 
-  base::WaitableEvent closed_event_1(true, false);
-  controller->Close(base::Bind(&base::WaitableEvent::Signal,
-                               base::Unretained(&closed_event_1)));
+  controller->Close(MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
 
-  base::WaitableEvent closed_event_2(true, false);
-  controller->Close(base::Bind(&base::WaitableEvent::Signal,
-                               base::Unretained(&closed_event_2)));
-
-  closed_event_1.Wait();
-  closed_event_2.Wait();
+  controller->Close(MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
 }
 
 }  // namespace media