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

Issue 6157007: Use the lock when accessing the buffer object.... (Closed)

Created:
9 years, 11 months ago by Chris Evans
Modified:
7 years, 2 months ago
CC:
chromium-reviews, vrk (LEFT CHROMIUM), sjl, Alpha Left Google, ddorwin+watch_chromium.org, acolwell GONE FROM CHROMIUM, annacc, awong
Visibility:
Public.

Description

Use the lock when accessing the buffer object. BUG=69195 TEST=play Z-Type for hours :) Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71211

Patch Set 1 #

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

Messages

Total messages: 11 (0 generated)
Chris Evans
Headache to track down, easy to fix :)
9 years, 11 months ago (2011-01-12 06:45:08 UTC) #1
Alpha Left Google
http://codereview.chromium.org/6157007/diff/1/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): http://codereview.chromium.org/6157007/diff/1/media/audio/audio_output_controller.cc#newcode231 media/audio/audio_output_controller.cc:231: AutoLock auto_lock(lock_); LGTM. Good catch!
9 years, 11 months ago (2011-01-12 06:47:52 UTC) #2
scherkus (not reviewing)
oh my :( I wonder if threadsanitizer could catch this...
9 years, 11 months ago (2011-01-12 20:48:41 UTC) #3
evanm
+thread race experts
9 years, 11 months ago (2011-01-12 21:38:31 UTC) #4
kcc2
> I wonder if threadsanitizer could catch this... It should, but only if there are ...
9 years, 11 months ago (2011-01-13 06:02:53 UTC) #5
scherkus (not reviewing)
On 2011/01/13 06:02:53, kcc2 wrote: > > I wonder if threadsanitizer could catch this... > ...
9 years, 11 months ago (2011-01-13 06:29:47 UTC) #6
kcc2
What is the object on which you had a race? Was it AudioOutputController::buffer_? What was ...
9 years, 11 months ago (2011-01-13 06:34:59 UTC) #7
kcc2
Voila. ==9818== WARNING: Possible data race during read of size 8 at 0x1D8CB4B0: {{{ ==9818== ...
9 years, 11 months ago (2011-01-13 08:54:01 UTC) #8
scherkus (not reviewing)
Cool! Yeah I have some ideas on cleaning up the code + improving testing. On ...
9 years, 11 months ago (2011-01-13 22:36:12 UTC) #9
Timur Iskhodzhanov
Do you mean "the tests are not run anywhere" or "the tests are not run ...
9 years, 11 months ago (2011-01-18 15:17:53 UTC) #10
scherkus (not reviewing)
9 years, 11 months ago (2011-01-18 18:23:47 UTC) #11
On 2011/01/18 15:17:53, Timur Iskhodzhanov wrote:
> Do you mean "the tests are not run anywhere" or "the tests are not run on the
> machines with no soundcards" ?

None of the tests in audio_output_controller_unittest.cc run because it checks
for CHROME_HEADLESS environment variable, which I believe  every bot defines
whether it has a sound card or not.

Powered by Google App Engine
This is Rietveld 408576698