Fix race condition in audio output controller and audio sync reader.

Reader can be called after controller stopped, causing hang and/or crash.
Fix consits of 2 parts:
(1) In low latency mode output controller should check its state, exactly like
in the high latency mode, and do not call reader if it is not playing anymore.
(2) Reader should not touch socket after it is closed, and socket-related
operations should be under the lock to avoid race with Close().

BUG=chromium-os:21130
BUG=chromium-os:17357

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

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

3 files changed, 24 insertions, 2 deletions

diff --git a/content/browser/renderer_host/media/audio_sync_reader.cc b/content/browser/renderer_host/media/audio_sync_reader.cc
index f9c8a4a..7e21cf1 100644
--- a/content/browser/renderer_host/media/audio_sync_reader.cc
+++ b/content/browser/renderer_host/media/audio_sync_reader.cc
@@ -36,7 +36,10 @@ void AudioSyncReader::UpdatePendingBytes(uint32 bytes) {
         shared_memory_,
         media::PacketSizeSizeInBytes(shared_memory_->created_size()));
   }
-  socket_->Send(&bytes, sizeof(bytes));
+  base::AutoLock auto_lock(lock_);
+  if (socket_.get()) {
+    socket_->Send(&bytes, sizeof(bytes));
+  }
 }
 
 uint32 AudioSyncReader::Read(void* data, uint32 size) {
@@ -81,7 +84,11 @@ uint32 AudioSyncReader::Read(void* data, uint32 size) {
 }
 
 void AudioSyncReader::Close() {
-  socket_->Close();
+  base::AutoLock auto_lock(lock_);
+  if (socket_.get()) {
+    socket_->Close();
+    socket_.reset(NULL);
+  }
 }
 
 bool AudioSyncReader::Init() {
diff --git a/content/browser/renderer_host/media/audio_sync_reader.h b/content/browser/renderer_host/media/audio_sync_reader.h
index 6bf9385..809e117 100644
--- a/content/browser/renderer_host/media/audio_sync_reader.h
+++ b/content/browser/renderer_host/media/audio_sync_reader.h
@@ -9,6 +9,7 @@
 #include "base/file_descriptor_posix.h"
 #include "base/process.h"
 #include "base/sync_socket.h"
+#include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "media/audio/audio_output_controller.h"
 
@@ -51,6 +52,12 @@ class AudioSyncReader : public media::AudioOutputController::SyncReader {
   // PrepareForeignSocketHandle() is called and ran successfully.
   scoped_ptr<base::SyncSocket> foreign_socket_;
 
+  // Protect socket_ access by lock to prevent race condition when audio
+  // controller thread closes the reader and hardware audio thread is reading
+  // data. This way we know that socket would not be deleted while we are
+  // writing data to it.
+  base::Lock lock_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioSyncReader);
 };
 
diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index 3331701..7118eaa 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -323,6 +323,14 @@ uint32 AudioOutputController::OnMoreData(
   }
 
   // Low latency mode.
+  {
+    // Check state and do nothing if we are not playing.
+    // We are on the hardware audio thread, so lock is needed.
+    base::AutoLock auto_lock(lock_);
+    if (state_ != kPlaying) {
+      return 0;
+    }
+  }
   uint32 size =  sync_reader_->Read(dest, max_size);
   sync_reader_->UpdatePendingBytes(buffers_state.total_bytes() + size);
   return size;