From f340282075194fc5d75e64c500ae6593c3792d8d Mon Sep 17 00:00:00 2001 From: "acolwell@chromium.org" Date: Thu, 23 Dec 2010 03:38:13 +0000 Subject: Fix AudioManager shutdown This patch makes sure the audio thread is stopped before the AudioManager is destroyed. Under heavy load it is possible to have tasks still on the audio thread's message loop when the audio manager is destroyed. These were getting called when the Thread object is destroyed which happens after all of the AudioManager destructors are called. The new Cleanup() method stops the thread before any destruction occurs. BUG=67804 TEST=none Review URL: http://codereview.chromium.org/6026011 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70034 0039d316-1c4b-4281-b951-d872f2087c98 --- media/audio/audio_manager.cc | 10 ++++++++-- media/audio/audio_manager.h | 3 +++ media/audio/audio_manager_base.cc | 8 ++++++++ media/audio/audio_manager_base.h | 1 + media/audio/audio_output_controller.cc | 9 +++++++++ media/audio/audio_output_proxy_unittest.cc | 1 + 6 files changed, 30 insertions(+), 2 deletions(-) diff --git a/media/audio/audio_manager.cc b/media/audio/audio_manager.cc index 3abd010..bc91c63 100644 --- a/media/audio/audio_manager.cc +++ b/media/audio/audio_manager.cc @@ -9,6 +9,7 @@ namespace { +bool g_destroy_called = false; AudioManager* g_audio_manager = NULL; // NullAudioManager is the audio manager used on the systems that have no @@ -40,13 +41,18 @@ class NullAudioManager : public AudioManager { // static void AudioManager::Destroy(void* not_used) { - delete g_audio_manager; + g_destroy_called = true; + + g_audio_manager->Cleanup(); + + AudioManager* audio_manager = g_audio_manager; g_audio_manager = NULL; + delete audio_manager; } // static AudioManager* AudioManager::GetAudioManager() { - if (!g_audio_manager) { + if (!g_audio_manager && !g_destroy_called) { g_audio_manager = CreateAudioManager(); g_audio_manager->Init(); base::AtExitManager::RegisterCallback(&AudioManager::Destroy, NULL); diff --git a/media/audio/audio_manager.h b/media/audio/audio_manager.h index aa7f018..2afc789 100644 --- a/media/audio/audio_manager.h +++ b/media/audio/audio_manager.h @@ -93,6 +93,9 @@ class AudioManager { // Called from GetAudioManager() to initialiaze the instance. virtual void Init() = 0; + // Called by Destroy() to cleanup the instance before destruction. + virtual void Cleanup() = 0; + private: static void Destroy(void*); diff --git a/media/audio/audio_manager_base.cc b/media/audio/audio_manager_base.cc index e522240..73a97ba 100644 --- a/media/audio/audio_manager_base.cc +++ b/media/audio/audio_manager_base.cc @@ -16,12 +16,20 @@ AudioManagerBase::AudioManagerBase() } AudioManagerBase::~AudioManagerBase() { + DCHECK(!audio_thread_.IsRunning()); } void AudioManagerBase::Init() { initialized_ = audio_thread_.Start(); } +void AudioManagerBase::Cleanup() { + if (initialized_) { + initialized_ = false; + audio_thread_.Stop(); + } +} + string16 AudioManagerBase::GetAudioInputDeviceModel() { return string16(); } diff --git a/media/audio/audio_manager_base.h b/media/audio/audio_manager_base.h index aa664f4..370a20c 100644 --- a/media/audio/audio_manager_base.h +++ b/media/audio/audio_manager_base.h @@ -18,6 +18,7 @@ class AudioManagerBase : public AudioManager { AudioManagerBase(); virtual void Init(); + virtual void Cleanup(); virtual MessageLoop* GetMessageLoop(); diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc index ee2614e..1d84638 100644 --- a/media/audio/audio_output_controller.cc +++ b/media/audio/audio_output_controller.cc @@ -48,6 +48,9 @@ scoped_refptr AudioOutputController::Create( if (!CheckParameters(params)) return NULL; + if (!AudioManager::GetAudioManager()) + return NULL; + // Starts the audio controller thread. scoped_refptr controller(new AudioOutputController( event_handler, buffer_capacity, NULL)); @@ -72,6 +75,9 @@ scoped_refptr AudioOutputController::CreateLowLatency( if (!CheckParameters(params)) return NULL; + if (!AudioManager::GetAudioManager()) + return NULL; + // Starts the audio controller thread. scoped_refptr controller(new AudioOutputController( event_handler, 0, sync_reader)); @@ -142,6 +148,9 @@ void AudioOutputController::DoCreate(AudioParameters params) { return; DCHECK(state_ == kEmpty); + if (!AudioManager::GetAudioManager()) + return; + stream_ = AudioManager::GetAudioManager()->MakeAudioOutputStreamProxy(params); if (!stream_) { // TODO(hclam): Define error types. diff --git a/media/audio/audio_output_proxy_unittest.cc b/media/audio/audio_output_proxy_unittest.cc index efc164c..0e531dd 100644 --- a/media/audio/audio_output_proxy_unittest.cc +++ b/media/audio/audio_output_proxy_unittest.cc @@ -38,6 +38,7 @@ class MockAudioManager : public AudioManager { MockAudioManager() { }; MOCK_METHOD0(Init, void()); + MOCK_METHOD0(Cleanup, void()); MOCK_METHOD0(HasAudioOutputDevices, bool()); MOCK_METHOD0(HasAudioInputDevices, bool()); MOCK_METHOD0(GetAudioInputDeviceModel, string16()); -- cgit v1.1