Add lock to AUAudioOutputStream::Start() and Stop().

Crash reports show a NULL pointer exeception occuring in Render(),
which should only occur if Render() is continuing to execute before
Start() or after Stop().  Lock and check to avoid crashing.

BUG=178765
TEST=none

Review URL: https://codereview.chromium.org/12384003

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

2 files changed, 31 insertions, 11 deletions

diff --git a/media/audio/mac/audio_low_latency_output_mac.cc b/media/audio/mac/audio_low_latency_output_mac.cc
index 98182b0..6ab0ead 100644
--- a/media/audio/mac/audio_low_latency_output_mac.cc
+++ b/media/audio/mac/audio_low_latency_output_mac.cc
@@ -203,19 +203,21 @@ void AUAudioOutputStream::Start(AudioSourceCallback* callback) {
   }
 
   stopped_ = false;
-  source_ = callback;
+  {
+    base::AutoLock auto_lock(source_lock_);
+    source_ = callback;
+  }
 
   AudioOutputUnitStart(output_unit_);
 }
 
 void AUAudioOutputStream::Stop() {
-  // We request a synchronous stop, so the next call can take some time. In
-  // the windows implementation we block here as well.
   if (stopped_)
     return;
 
   AudioOutputUnitStop(output_unit_);
 
+  base::AutoLock auto_lock(source_lock_);
   source_ = NULL;
   stopped_ = true;
 }
@@ -254,14 +256,27 @@ OSStatus AUAudioOutputStream::Render(UInt32 number_of_frames,
   // or smaller than the value set during Configure().  In this case either
   // audio input or audio output will be broken, so just output silence.
   // TODO(crogers): Figure out what can trigger a change in |number_of_frames|.
-  // See http://crbug.com/1543 for details.
-   if (number_of_frames != static_cast<UInt32>(audio_bus_->frames())) {
-     memset(audio_data, 0, number_of_frames * format_.mBytesPerFrame);
-     return noErr;
-   }
-
-  int frames_filled = source_->OnMoreData(
-      audio_bus_.get(), AudioBuffersState(0, hardware_pending_bytes));
+  // See http://crbug.com/154352 for details.
+  if (number_of_frames != static_cast<UInt32>(audio_bus_->frames())) {
+    memset(audio_data, 0, number_of_frames * format_.mBytesPerFrame);
+    return noErr;
+  }
+
+  int frames_filled = 0;
+  {
+    // Render() shouldn't be called except between AudioOutputUnitStart() and
+    // AudioOutputUnitStop() calls, but crash reports have shown otherwise:
+    // http://crbug.com/178765.  We use |source_lock_| to prevent races and
+    // crashes in Render() when |source_| is cleared.
+    base::AutoLock auto_lock(source_lock_);
+    if (!source_) {
+      memset(audio_data, 0, number_of_frames * format_.mBytesPerFrame);
+      return noErr;
+    }
+
+    frames_filled = source_->OnMoreData(
+        audio_bus_.get(), AudioBuffersState(0, hardware_pending_bytes));
+  }
 
   // Note: If this ever changes to output raw float the data must be clipped and
   // sanitized since it may come from an untrusted source such as NaCl.
diff --git a/media/audio/mac/audio_low_latency_output_mac.h b/media/audio/mac/audio_low_latency_output_mac.h
index 4ceb4af..27f3b3a 100644
--- a/media/audio/mac/audio_low_latency_output_mac.h
+++ b/media/audio/mac/audio_low_latency_output_mac.h
@@ -21,6 +21,7 @@
 #include <CoreAudio/CoreAudio.h>
 
 #include "base/compiler_specific.h"
+#include "base/synchronization/lock.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_parameters.h"
 
@@ -81,6 +82,10 @@ class AUAudioOutputStream : public AudioOutputStream {
   // Pointer to the object that will provide the audio samples.
   AudioSourceCallback* source_;
 
+  // Protects |source_|.  Necessary since Render() calls seem to be in flight
+  // when |output_unit_| is supposedly stopped.  See http://crbug.com/178765.
+  base::Lock source_lock_;
+
   // Structure that holds the stream format details such as bitrate.
   AudioStreamBasicDescription format_;