Stop the AudioDeviceThread when the IO loop goes away.

content/renderer/media/audio_device.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/audio_device.h"
 
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
@@ skipping 81 matching lines @@
   CHECK_EQ(0, stream_id_);
 }
 
 void AudioDevice::Start() {
   DCHECK(callback_) << "Initialize hasn't been called";
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioDevice::InitializeOnIOThread, this, audio_parameters_));
 }
 
 void AudioDevice::Stop() {
   audio_thread_.Stop(MessageLoop::current());

[scherkus (not reviewing), 2012/02/29 21:10:15]

remind me again.. what's the advantage to stopping the audio thread on current
thread as opposed to the IO thread? (see my other comment below)

[tommi (sloooow) - chröme, 2012/02/29 21:22:13]

The IO thread isn't allowed to join threads.

However, we're in a bit of a pickle with the pipeline thread so we have to break
that rule below basically because we're out of threads to wait on.

 
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioDevice::ShutDownOnIOThread, this));
 }
 
 void AudioDevice::Play() {
   message_loop()->PostTask(FROM_HERE,
       base::Bind(&AudioDevice::PlayOnIOThread, this));
 }
 
@@ skipping 58 matching lines @@
 
   // Make sure we don't call shutdown more than once.
   if (stream_id_) {
     is_started_ = false;
 
     filter_->RemoveDelegate(stream_id_);
     Send(new AudioHostMsg_CloseStream(stream_id_));
     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.
+  audio_thread_.Stop(MessageLoop::current());

[scherkus (not reviewing), 2012/02/29 21:10:15]

hold the phone...

combined with the change to WillDestroyCurrentMessageLoop() to synchronously
stop audio_thread_... perhaps we should always synchronously stop audio_thread_
inside of ShutDownOnIOThread()?

Here's my thinking... there are two spots where ShutDownOnIOThread can get
called:
  1) Stop() via PostTask
  2) WillDestroyCurrentMessageLoop()

In (1) we already call audio_thread_.Stop(MessageLoop::current()) prior to
posting the task for ShutDownOnIOThread(). If it doesn't matter which thread is
used to stop audio_thread_, then why not always stop it inside this method?
(hopefully I'm not missing anything here!)

If that all makes sense I wonder if audio_thread_.Stop() even needs to accept a
MessageLoop parameter anymore if synchronous stopping is always used.

[tommi (sloooow) - chröme, 2012/02/29 21:22:13]

We shouldn't do that on the IO thread due to thread restrictions.  However, when
we hit the WillDestroyCurrentMessageLoop case, that means that the Pipeline
thread is still running while the IO is ending and as we've seen in crash
reports, the main thread has also gone.  That's why I added the synchronous wait
there.  It would be good if the pipeline thread has been properly terminated
before the main and io threads go down.

   audio_callback_.reset();
 }
 
 void AudioDevice::SetVolumeOnIOThread(double volume) {
   DCHECK(message_loop()->BelongsToCurrentThread());
   if (stream_id_)
     Send(new AudioHostMsg_SetVolume(stream_id_, volume));
 }
 
 void AudioDevice::OnStateChanged(AudioStreamState state) {
@@ skipping 20 matching lines @@
 #endif
 
   // Takes care of the case when Stop() is called before OnStreamCreated().
   if (!stream_id_) {
     base::SharedMemory::CloseHandle(handle);
     // Close the socket handler.
     base::SyncSocket socket(socket_handle);
     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");
 
   // We handle the case where Play() and/or Pause() may have been called
   // multiple times before OnStreamCreated() gets called.
   is_started_ = true;
   if (play_on_start_)
     PlayOnIOThread();
 }
 
 void AudioDevice::Send(IPC::Message* message) {
   filter_->Send(message);
 }
 
 void AudioDevice::WillDestroyCurrentMessageLoop() {
+  LOG(ERROR) << "IO loop going away before the audio device has been stopped";
+  audio_thread_.Stop(NULL);
   ShutDownOnIOThread();
 }
 
 // AudioDevice::AudioThreadCallback
 
 AudioDevice::AudioThreadCallback::AudioThreadCallback(
     const AudioParameters& audio_parameters,
     base::SharedMemoryHandle memory,
     int memory_length,
     media::AudioRendererSink::RenderCallback* render_callback)
@@ skipping 31 matching lines @@
   // to the browser process as float, so we don't lose precision for
   // audio hardware which has better than 16bit precision.
   int16* data = reinterpret_cast<int16*>(shared_memory_.memory());
   media::InterleaveFloatToInt16(audio_data_, data,
       audio_parameters_.samples_per_packet);
 
   // Let the host know we are done.
   media::SetActualDataSizeInBytes(&shared_memory_, memory_length_,
       num_frames * audio_parameters_.channels * sizeof(data[0]));
 }