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

Issue 6878081: Blind fix for a Linux only crash in AudioManager::Destroy (Closed)

Created:
9 years, 8 months ago by Lei Zhang
Modified:
9 years, 7 months ago
CC:
chromium-reviews, hclam+watch_chromium.org, sjl, ddorwin+watch_chromium.org, fischman+watch_chromium.org, acolwell GONE FROM CHROMIUM, annacc, ajwong+watch_chromium.org, vrk (LEFT CHROMIUM), scherkus (not reviewing)
Visibility:
Public.

Description

Blind fix for a Linux only crash in AudioManager::Destroy BUG=80059 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82612

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -4 lines) Patch
M media/audio/audio_manager.cc View 1 chunk +6 lines, -4 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Lei Zhang
9 years, 8 months ago (2011-04-20 21:08:44 UTC) #1
scherkus (not reviewing)
looking at the code it seems impossible for g_audio_manager to be non-NULL inside Destroy does ...
9 years, 8 months ago (2011-04-21 18:17:17 UTC) #2
Lei Zhang
On 2011/04/21 18:17:17, scherkus wrote: > looking at the code it seems impossible for g_audio_manager ...
9 years, 8 months ago (2011-04-22 00:22:21 UTC) #3
scherkus (not reviewing)
crazy! alright LGTM and hopefully that fixes it :|
9 years, 8 months ago (2011-04-22 02:12:21 UTC) #4
Ami GONE FROM WEBRTC_CHROMIUM
9 years, 8 months ago (2011-04-23 21:04:45 UTC) #5
FYI

Random thoughts about the underlying bug:
- GetAudioManager can race with itself, registering multiple AtExit handlers
(and incidentally leaking audio managers).  The second AtExit invocation
could cause the reported crash.
- GAM & Destroy have no common memory barriers so unless the AtExit
machinery makes such promises, it's quite possible for the observed state of
the global variables to be inconsistent.

-a

Powered by Google App Engine
This is Rietveld 408576698