New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 638578 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Data races in WebRtcMediaRecorderTest.MediaRecorderStartAndDataAvailable/0

Project Member Reported by glider@chromium.org, Aug 17 2016

Issue description

Accidentally found the following reports in the trybot log at https://chromium-swarm.appspot.com/user/task/30b126bf3b5cac10:

[ RUN      ] WebRtcMediaRecorderTest.MediaRecorderStartAndDataAvailable/0
[16698:16698:0817/051450:9226901585:WARNING:audio_manager.cc(317)] Multiple instances of AudioManager detected
[16698:16698:0817/051450:9226901787:WARNING:audio_manager.cc(278)] Multiple instances of AudioManager detected
Xlib:  extension "RANDR" missing on display ":9".
[16698:16698:0817/051458:9235113201:INFO:CONSOLE(90)] "Recorder object created, mimeType = video/webm;codecs=VP8", source: http://127.0.0.1:35868/media/mediarecorder_test.html (90)
[16698:16698:0817/051458:9235140110:INFO:CONSOLE(22)] "Waiting for", source: http://127.0.0.1:35868/media/mediarecorder_test_utils.js (22)
[16698:16722:0817/051459:9236028059:WARNING:audio_input_sync_writer.cc(241)] AISW: No room in fifo.
...
==================
WARNING: ThreadSanitizer: data race (pid=16742)
  Read of size 4 at 0x7d5000006110 by thread T10 (mutexes: write M694255126658995112, write M710299148791867624):
    #0 base::Thread::task_runner() const base/threading/thread.h:207:5 (content_browsertests+0x0000006d6c2f)
    #1 content::AudioTrackRecorder::OnSetFormat(media::AudioParameters const&) content/renderer/media/audio_track_recorder.cc:314:19 (content_browsertests+0x00000449257e)
    #2 content::MediaStreamAudioDeliverer<content::MediaStreamAudioSink>::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_deliverer.h:114:19 (content_browsertests+0x000004236cef)
    #3 content::MediaStreamAudioTrack::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_track.cc:130:16 (content_browsertests+0x000004236b58)
    #4 content::MediaStreamAudioDeliverer<content::MediaStreamAudioTrack>::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_deliverer.h:122:17 (content_browsertests+0x000004234025)
    #5 content::MediaStreamAudioSource::DeliverDataToTracks(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_source.cc:117:14 (content_browsertests+0x000004233d70)
    #6 content::ProcessedLocalAudioSource::Capture(media::AudioBus const*, int, double, bool) content/renderer/media/webrtc/processed_local_audio_source.cc:328:29 (content_browsertests+0x000004368693)
    #7 non-virtual thunk to content::ProcessedLocalAudioSource::Capture(media::AudioBus const*, int, double, bool) content/renderer/media/webrtc/processed_local_audio_source.cc:266:33 (content_browsertests+0x000004368726)
    #8 media::AudioInputDevice::AudioThreadCallback::Process(unsigned int) media/audio/audio_input_device.cc:345:22 (content_browsertests+0x000000c44b12)
    #9 media::AudioDeviceThread::ThreadMain() media/audio/audio_device_thread.cc:84:18 (content_browsertests+0x000000c3d08e)
    #10 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:71:13 (content_browsertests+0x000002cc0168)

  Previous write of size 4 at 0x7d5000006110 by thread T11:
    #0 base::Thread::ThreadMain() base/threading/thread.cc:266:7 (content_browsertests+0x000002cc765d)
    #1 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:71:13 (content_browsertests+0x000002cc0168)

  Location is heap block of size 472 at 0x7d5000006000 allocated by main thread:
    #0 operator new(unsigned long) <null> (content_browsertests+0x00000050f8f2)
    #1 content::MediaRecorderHandler::start(int) content/renderer/media/media_recorder_handler.cc:220:32 (content_browsertests+0x00000430b8e6)
    #2 blink::MediaRecorder::start(int, blink::ExceptionState&) third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:187:29 (content_browsertests+0x00000641abfa)
    #3 blink::MediaRecorder::start(blink::ExceptionState&) third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:176:5 (content_browsertests+0x00000641ab02)
    #4 startMethod out/Release/gen/blink/bindings/modules/v8/V8MediaRecorder.cpp:314:19 (content_browsertests+0x000006094d6f)
    #5 blink::MediaRecorderV8Internal::startMethodCallback(v8::FunctionCallbackInfo<v8::Value> const&) out/Release/gen/blink/bindings/modules/v8/V8MediaRecorder.cpp:334 (content_browsertests+0x000006094d6f)
    #6 v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) v8/src/api-arguments.cc:21:3 (content_browsertests+0x000000e7b5ad)
    #7 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) v8/src/builtins/builtins-api.cc:106:36 (content_browsertests+0x000000f4fd90)
    #8 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) v8/src/builtins/builtins-api.cc:135:5 (content_browsertests+0x000000f4e49a)
    #9 v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) v8/src/builtins/builtins-api.cc:123:1 (content_browsertests+0x000000f4e0aa)
    #10 <null> <null> (0x7fef7df063a7)
    #11 Call v8/src/execution.cc:176:10 (content_browsertests+0x00000141b2a9)
    #12 v8::internal::Execution::TryCall(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::MaybeHandle<v8::internal::Object>*) v8/src/execution.cc:214 (content_browsertests+0x00000141b2a9)
    #13 v8::internal::Isolate::RunMicrotasksInternal() v8/src/isolate.cc:3001:5 (content_browsertests+0x000001602f5c)
    #14 v8::internal::Isolate::RunMicrotasks() v8/src/isolate.cc:2985:3 (content_browsertests+0x000001602289)
    #15 v8::MicrotasksScope::PerformCheckpoint(v8::Isolate*) v8/src/api.cc:8089:14 (content_browsertests+0x000000ec1f4e)
    #16 blink::Microtask::performCheckpoint(v8::Isolate*) third_party/WebKit/Source/bindings/core/v8/Microtask.cpp:42:5 (content_browsertests+0x0000048fb5c8)
    #17 blink::(anonymous namespace)::EndOfTaskRunner::didProcessTask() third_party/WebKit/Source/web/WebKit.cpp:67:9 (content_browsertests+0x000004812d39)
    #18 blink::scheduler::WebThreadBase::TaskObserverAdapter::DidProcessTask(base::PendingTask const&) third_party/WebKit/Source/platform/scheduler/child/webthread_base.cc:32:16 (content_browsertests+0x000004642e4f)
    #19 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue*, blink::scheduler::internal::TaskQueueImpl::Task*) third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:325:5 (content_browsertests+0x000004731fcb)
    #20 blink::scheduler::TaskQueueManager::DoWork(base::TimeTicks, bool) third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:218:13 (content_browsertests+0x00000472fc73)
    #21 Invoke<const base::WeakPtr<blink::scheduler::TaskQueueManager> &, const base::TimeTicks &, const bool &> base/bind_internal.h:214:12 (content_browsertests+0x0000047331d7)
    #22 MakeItSo<void (blink::scheduler::TaskQueueManager::*const &)(base::TimeTicks, bool), const base::WeakPtr<blink::scheduler::TaskQueueManager> &, const base::TimeTicks &, const bool &> base/bind_internal.h:303 (content_browsertests+0x0000047331d7)
    #23 RunImpl<void (blink::scheduler::TaskQueueManager::*const &)(base::TimeTicks, bool), const std::__1::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>, base::TimeTicks, bool> &, 0, 1, 2> base/bind_internal.h:346 (content_browsertests+0x0000047331d7)
    #24 base::internal::Invoker<base::internal::BindState<void (blink::scheduler::TaskQueueManager::*)(base::TimeTicks, bool), base::WeakPtr<blink::scheduler::TaskQueueManager>, base::TimeTicks, bool>, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:324 (content_browsertests+0x0000047331d7)
    #25 Run base/callback.h:388:12 (content_browsertests+0x000002d11be9)
    #26 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask const&) base/debug/task_annotator.cc:54 (content_browsertests+0x000002d11be9)
    #27 base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:488:19 (content_browsertests+0x000002c6227c)
    #28 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) base/message_loop/message_loop.cc:497:5 (content_browsertests+0x000002c6289d)
    #29 base::MessageLoop::DoWork() base/message_loop/message_loop.cc:621:13 (content_browsertests+0x000002c63054)
    #30 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_default.cc:35:31 (content_browsertests+0x000002c66fc5)
    #31 base::MessageLoop::RunHandler() base/message_loop/message_loop.cc:451:10 (content_browsertests+0x000002c61ae0)
    #32 base::RunLoop::Run() base/run_loop.cc:35:10 (content_browsertests+0x000002c95dde)
    #33 content::RendererMain(content::MainFunctionParams const&) content/renderer/renderer_main.cc:198:23 (content_browsertests+0x0000042d19cd)
    #34 content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:343:14 (content_browsertests+0x000001f3c312)
    #35 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:426:12 (content_browsertests+0x000001f3d07d)
    #36 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:785:12 (content_browsertests+0x000001f3de5d)
    #37 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:28 (content_browsertests+0x000001f33fce)
    #38 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:523:12 (content_browsertests+0x0000027d5009)
    #39 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x0000027b79d2)

  Mutex M694255126658995112 is already destroyed.

  Mutex M710299148791867624 is already destroyed.
...
==================
WARNING: ThreadSanitizer: data race (pid=16742)
  Read of size 8 at 0x7d480000bb58 by thread T10:
    #0 operator-> base/memory/ref_counted.h:323:12 (content_browsertests+0x0000044988b1)
    #1 content::MediaStreamAudioProcessor::ProcessData(float const* const*, int, base::TimeDelta, int, bool, float* const*) content/renderer/media/media_stream_audio_processor.cc:812 (content_browsertests+0x0000044988b1)
    #2 content::MediaStreamAudioProcessor::ProcessAndConsumeData(int, bool, media::AudioBus**, base::TimeDelta*, int*) content/renderer/media/media_stream_audio_processor.cc:375:19 (content_browsertests+0x000004497c15)
    #3 content::ProcessedLocalAudioSource::Capture(media::AudioBus const*, int, double, bool) content/renderer/media/webrtc/processed_local_audio_source.cc:321:28 (content_browsertests+0x0000043685e2)
    #4 non-virtual thunk to content::ProcessedLocalAudioSource::Capture(media::AudioBus const*, int, double, bool) content/renderer/media/webrtc/processed_local_audio_source.cc:266:33 (content_browsertests+0x000004368726)
    #5 media::AudioInputDevice::AudioThreadCallback::Process(unsigned int) media/audio/audio_input_device.cc:345:22 (content_browsertests+0x000000c44b12)
    #6 media::AudioDeviceThread::ThreadMain() media/audio/audio_device_thread.cc:84:18 (content_browsertests+0x000000c3d08e)
    #7 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:71:13 (content_browsertests+0x000002cc0168)

  Previous write of size 8 at 0x7d480000bb58 by main thread:
    #0 swap base/memory/ref_counted.h:360:9 (content_browsertests+0x000002c61928)
    #1 swap base/memory/ref_counted.h:364 (content_browsertests+0x000002c61928)
    #2 operator= base/memory/ref_counted.h:347 (content_browsertests+0x000002c61928)
    #3 base::MessageLoop::SetTaskRunner(scoped_refptr<base::SingleThreadTaskRunner>) base/message_loop/message_loop.cc:436 (content_browsertests+0x000002c61928)
    #4 blink::scheduler::SchedulerTqmDelegateImpl::RestoreDefaultTaskRunner() third_party/WebKit/Source/platform/scheduler/child/scheduler_tqm_delegate_impl.cc:38:20 (content_browsertests+0x00000470f3d6)
    #5 blink::scheduler::SchedulerHelper::Shutdown() third_party/WebKit/Source/platform/scheduler/child/scheduler_helper.cc:62:33 (content_browsertests+0x00000470e738)
    #6 blink::scheduler::RendererSchedulerImpl::Shutdown() third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:167:11 (content_browsertests+0x000004646b95)
    #7 content::RenderThreadImpl::Shutdown() content/renderer/render_thread_impl.cc:973:24 (content_browsertests+0x000004288745)
    #8 non-virtual thunk to content::RenderThreadImpl::Shutdown() content/renderer/render_thread_impl.cc:870:24 (content_browsertests+0x00000428896d)
    #9 content::ChildProcess::~ChildProcess() content/child/child_process.cc:73:19 (content_browsertests+0x00000415f52b)
    #10 content::RenderProcessImpl::~RenderProcessImpl() content/renderer/render_process_impl.cc:113:1 (content_browsertests+0x000004281c4f)
    #11 content::RendererMain(content::MainFunctionParams const&) content/renderer/renderer_main.cc:207:3 (content_browsertests+0x0000042d1a1b)
    #12 content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:343:14 (content_browsertests+0x000001f3c312)
    #13 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:426:12 (content_browsertests+0x000001f3d07d)
    #14 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:785:12 (content_browsertests+0x000001f3de5d)
    #15 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:28 (content_browsertests+0x000001f33fce)
    #16 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:523:12 (content_browsertests+0x0000027d5009)
    #17 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x0000027b79d2)

  Location is heap block of size 368 at 0x7d480000ba00 allocated by main thread:
    #0 operator new(unsigned long) <null> (content_browsertests+0x00000050f8f2)
    #1 content::RendererMain(content::MainFunctionParams const&) content/renderer/renderer_main.cc:140:56 (content_browsertests+0x0000042d185a)
    #2 content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:343:14 (content_browsertests+0x000001f3c312)
    #3 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:426:12 (content_browsertests+0x000001f3d07d)
    #4 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:785:12 (content_browsertests+0x000001f3de5d)
    #5 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:28 (content_browsertests+0x000001f33fce)
    #6 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:523:12 (content_browsertests+0x0000027d5009)
    #7 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x0000027b79d2)

  Thread T10 'AudioInputDevic' (tid=16832, running) created by thread T1 at:
    #0 pthread_create <null> (content_browsertests+0x0000004b06f5)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:110:13 (content_browsertests+0x000002cbfd05)
    #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:191:10 (content_browsertests+0x000002cbfbb5)
    #3 media::AudioDeviceThread::AudioDeviceThread(media::AudioDeviceThread::Callback*, int, char const*) media/audio/audio_device_thread.cc:51:3 (content_browsertests+0x000000c3ce06)
    #4 media::AudioInputDevice::OnStreamCreated(base::FileDescriptor, int, int, int) media/audio/audio_input_device.cc:151:27 (content_browsertests+0x000000c439a1)
    #5 non-virtual thunk to media::AudioInputDevice::OnStreamCreated(base::FileDescriptor, int, int, int) media/audio/audio_input_device.cc:123:24 (content_browsertests+0x000000c43a9d)
...

There are tens of similar reports in the log.
 

Comment 1 by ajha@chromium.org, Aug 22 2016

Components: Blink>MediaStream>Recording Blink>WebRTC

Comment 2 by mcasas@chromium.org, Aug 22 2016

Cc: -juberti@chromium.org mcasas@chromium.org
Components: -Blink>WebRTC
Owner: m...@chromium.org
Status: Assigned (was: Untriaged)
Seems like after ~AudioTrackRecorder(), we still get AudioTrackRecorder::AudioEncoder::OnSetFormat() calls on the |encoder_thread_|, which should not happen. Hmm, miu@ does this all ring a bell to you? 


    #1 content::AudioTrackRecorder::OnSetFormat(media::AudioParameters const&) content/renderer/media/audio_track_recorder.cc:314:19 (content_browsertests+0x00000449257e)
    #2 content::MediaStreamAudioDeliverer<content::MediaStreamAudioSink>::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_deliverer.h:114:19 (content_browsertests+0x000004236cef)
    #3 content::MediaStreamAudioTrack::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_track.cc:130:16 (content_browsertests+0x000004236b58)
    #4 content::MediaStreamAudioDeliverer<content::MediaStreamAudioTrack>::OnData(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_deliverer.h:122:17 (content_browsertests+0x000004234025)
    #5 content::MediaStreamAudioSource::DeliverDataToTracks(media::AudioBus const&, base::TimeTicks) content/renderer/media/media_stream_audio_source.cc:117:14 (content_browsertests+0x000004233d70)
    #6 content::WebAudioMediaStreamSource::DeliverRebufferedAudio(media::AudioBus const&, int) content/renderer/media/webaudio_media_stream_source.cc:108:27 (content_browsertests+0x00000447238c)

Cc: niklase@chromium.org

Comment 4 by mcasas@chromium.org, Aug 29 2016

Labels: -Pri-1 Pri-2

Comment 5 by m...@chromium.org, Aug 29 2016

Owner: mcasas@chromium.org
You have to call MediaStreamAudioSink::Stop() before the destructor. The stack trace shows the audio thread is still active.

Comment 6 by mcasas@chromium.org, Aug 30 2016

miu@: MediaStreamAudioSink::Stop() doesn't exist :)
Did you mean MediaStreamAudioTrack::Stop() ? In that case, the
Track would stop for all the clients, which is not what we want.

I thought about adding a Finalize() method to AudioTrackRecorder
to, well, finalize it before the destructor, but seeing how 
MediaStreamAudioSink::RemoveFromAudioTrack() is exercised [1], we
see that four classes call it from the destructor:

~CastAudioSink()
~AudioTrackRecorder()
~SpeechRecognitionAudioSink()
~WebRtcLocalAudioSourceProvider()

And two from specific methods:
TrackAudioRenderer::Stop()
PepperMediaStreamAudioTrackHost::OnClose()

I believe MediaStreamAudioSink::RemoveFromAudioTrack() does not 
guarantee no more frames delivered on IO thread so, are all these
classes that call it from destructor potentially having the 
same data race?

[1] https://cs.chromium.org/chromium/src/content/public/renderer/media_stream_audio_sink.h?sq=package:chromium&dr=CSs&rcl=1472570131&l=36

Comment 7 by m...@chromium.org, Sep 1 2016

Oops, I meant MediaStreamAudioSink::RemoveFromAudioTrack() not MediaStreamAudioSink::Stop(). It should be impossible for AudioTrackRecorder::OnSetFormat() or OnData() to be called after MediaStreamAudioSink::RemoveFromAudioTrack(). The MediaStreamAudioDeliverer utility class ensures this, even across threads.

Actually, looking at the first error in the log, this isn't happening at shutdown. It's happening at construction. Looking into this further...

Comment 8 by m...@chromium.org, Sep 1 2016

Components: Internals>Core
Owner: gab@chromium.org
Okay, I think I know exactly what's happening. It's a race condition at the time the AudioTrackRecorder is constructed (and NOT at shutdown!).

If you look at the line numbers in the stacks, you'll see TSAN is complaining about a data race where two threads are accessing the base::Thread::id_ member during (or shortly after) AudioTrackRecorder's constructor runs:

1. The audio thread is calling AudioTrackRecorder::OnSetFormat(), which calls encoder_thread_.task_runner(). The DCHECK() inside base::Thread::task_runner() is reading the |id_| field.

2. The encoder thread, at the top of base::Thread::ThreadMain(), is mutating the field: id_ = PlatformThread::CurrentId();

IMHO, the DCHECK() base::Thread::task_runner() is broken: The base::Thread class guarantees a message_loop_ instance exists once Thread::Start() returns, regardless of whether the thread has actually begun to run. Therefore, base::Thread::task_runner() should *not* be attempting to read the |id_| field.

Assigning to gab to take a look...

(HOWEVER, NOTE: There seem to be other TSAN errors unrelated to this problem lumped in this crbug, and should be addressed separately!)

Comment 9 by gab@chromium.org, Sep 2 2016

Cc: m...@chromium.org
Status: Started (was: Assigned)
The check in Thread is indeed racy, thanks for looping me in, fixed in https://codereview.chromium.org/2303943003/
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e09ce122b81dadacd6ffde73bee126952ff0d1f

commit 3e09ce122b81dadacd6ffde73bee126952ff0d1f
Author: gab <gab@chromium.org>
Date: Wed Sep 07 23:41:05 2016

Ensure that |Thread::id_| is only accessed when an happens-after relationship has been established with its write.

In practice this mostly means only reading it while |id_event_| is signaled.

Also don't AssertWaitAllowed() in WaitableEvent::TimedWait(0) on Windows to allow
WaitableEvent::IsSignaled() to be called on threads which aren't allowed to wait.
From MSDN:
  The time-out interval, in milliseconds. If a nonzero value is specified, the function
  waits until the object is signaled or the interval elapses. If dwMilliseconds is zero,
  the function does not enter a wait state if the object is not signaled; it always returns
  immediately. If dwMilliseconds is INFINITE, the function will return only when the object
  is signaled.

BUG= 638578 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng

Review-Url: https://codereview.chromium.org/2303943003
Cr-Commit-Position: refs/heads/master@{#417117}

[modify] https://crrev.com/3e09ce122b81dadacd6ffde73bee126952ff0d1f/base/synchronization/waitable_event_win.cc
[modify] https://crrev.com/3e09ce122b81dadacd6ffde73bee126952ff0d1f/base/threading/thread.cc
[modify] https://crrev.com/3e09ce122b81dadacd6ffde73bee126952ff0d1f/base/threading/thread.h

Comment 11 by gab@chromium.org, Sep 8 2016

Cc: -dvyukov@chromium.org gab@chromium.org
Owner: m...@chromium.org
Status: Assigned (was: Started)
Thread::id_ race fixed, back to miu@ per #8 : "(HOWEVER, NOTE: There seem to be other TSAN errors unrelated to this problem lumped in this crbug, and should be addressed separately!)"
Splitted the second issue into  issue 647178 

Comment 13 by gab@chromium.org, Sep 15 2016

Owner: gab@chromium.org
Status: Fixed (was: Assigned)
Thanks, marking this one as fixed then :-)
Cc: anatolid@chromium.org
Can the owner please set a milestone label to this issue?

FYI, the last CL associated with this issue has been added after the M54 branch was created and before the M55 branch was created, so perhaps it should be labelled as M55?

Comment 15 by gab@chromium.org, Oct 18 2016

Labels: M-55
Well it was fixed in 55, I think it was a long standing issue though.
Components: Blink>MediaRecording
Components: -Blink>MediaStream>Recording

Sign in to add a comment