Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1667)

Unified Diff: media/audio/audio_manager.cc

Issue 2934613002: Avoid shutdown crash if audio thread is hung. (Closed)
Patch Set: fixes recursive acquire Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | media/audio/audio_manager_base.cc » ('j') | media/audio/audio_manager_base.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/audio/audio_manager.cc
diff --git a/media/audio/audio_manager.cc b/media/audio/audio_manager.cc
index 87a2b121e5ca0f73d0e8199f59cc16e25b296013..298a5b523a59acedc983781a367f58d17fc72145 100644
--- a/media/audio/audio_manager.cc
+++ b/media/audio/audio_manager.cc
@@ -81,6 +81,11 @@ class AudioManagerHelper : public base::PowerObserver {
base::Unretained(this)));
}
+ bool IsAudioThreadHung() const {
+ base::AutoLock lock(hang_lock_);
+ return audio_thread_status_ == THREAD_HUNG;
+ }
+
base::SingleThreadTaskRunner* monitor_task_runner() const {
return monitor_task_runner_.get();
}
@@ -204,6 +209,7 @@ class AudioManagerHelper : public base::PowerObserver {
void HistogramThreadStatus(ThreadStatus status) {
DCHECK(monitor_task_runner_->BelongsToCurrentThread());
+ hang_lock_.AssertAcquired();
audio_thread_status_ = status;
UMA_HISTOGRAM_ENUMERATION("Media.AudioThreadStatus", audio_thread_status_,
THREAD_MAX + 1);
@@ -215,7 +221,7 @@ class AudioManagerHelper : public base::PowerObserver {
scoped_refptr<base::SingleThreadTaskRunner> monitor_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner_;
- base::Lock hang_lock_;
+ mutable base::Lock hang_lock_;
DaleCurtis 2017/06/13 23:43:15 Just don't make the method const.
alokp 2017/06/14 17:34:34 Done.
bool hang_detection_enabled_ = true;
base::TimeTicks last_audio_thread_timer_tick_;
uint32_t failed_pings_ = 0;
@@ -332,8 +338,6 @@ AudioManager* AudioManager::Get() {
void AudioManager::Shutdown() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- // TODO(alokp): Suspend hang monitor.
-
if (audio_thread_->GetTaskRunner()->BelongsToCurrentThread()) {
ShutdownOnAudioThread();
} else {
@@ -341,7 +345,13 @@ void AudioManager::Shutdown() {
FROM_HERE, base::Bind(&AudioManager::ShutdownOnAudioThread,
base::Unretained(this)));
}
- audio_thread_->Stop();
+
+ // Do not attempt to stop the audio thread if it is hung.
+ // Otherwise the current thread will hang too: crbug.com/729494
+ if (!GetHelper()->IsAudioThreadHung())
+ audio_thread_->Stop();
DaleCurtis 2017/06/13 23:43:15 ~audio_thread_ will still call stop now. You need
alokp 2017/06/14 17:34:35 Good catch. I followed your suggestion of leaking
+ // TODO(alokp): Suspend hang monitor.
+
shutdown_ = true;
}
« no previous file with comments | « no previous file | media/audio/audio_manager_base.cc » ('j') | media/audio/audio_manager_base.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698