Closed Bug 1429507 Opened 2 years ago Closed Last year

Crash near null [@ GraphRate]

Categories

(Core :: WebRTC, defect, P2, critical)

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 + wontfix
firefox60 - wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: jkratzer, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Attachments

(2 files)

Attached file trigger.html
Testcase found while fuzzing mozilla-central rev d5f42a23909e.

==16524==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x7ff95bde72b1 bp 0x7ff93fbf9d90 sp 0x7ff93fbf9cc0 T34)
==16524==The signal is caused by a READ memory access.
==16524==Hint: address points to the zero page.
    #0 0x7ff95bde72b0 in GraphRate /builds/worker/workspace/build/src/dom/media/StreamTracks.h:195:12
    #1 0x7ff95bde72b0 in GraphRate /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.h:278
    #2 0x7ff95bde72b0 in AddTrack /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.h:739
    #3 0x7ff95bde72b0 in mozilla::MediaEngineDefaultVideoSource::Start(mozilla::SourceMediaStream*, int, nsMainThreadPtrHandle<nsIPrincipal> const&) /builds/worker/workspace/build/src/dom/media/webrtc/MediaEngineDefault.cpp:187
    #4 0x7ff95b79bbe9 in mozilla::GetUserMediaStreamRunnable::Run()::{lambda()#1}::operator()() /builds/worker/workspace/build/src/dom/media/MediaManager.cpp:1227:44
    #5 0x7ff95b79b42c in mozilla::media::LambdaTask<mozilla::GetUserMediaStreamRunnable::Run()::{lambda()#1}>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/media/MediaTaskUtils.h:37:5
    #6 0x7ff95588c4fd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #7 0x7ff9558a7fb0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:517:10
    #8 0x7ff95673f845 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:364:5
    #9 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #10 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #11 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #12 0x7ff9566b156f in base::Thread::ThreadMain() /builds/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:181:16
    #13 0x7ff9566a2fdc in ThreadFunc(void*) /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:38:13
    #14 0x7ff9759aa6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #15 0x7ff974a2c3dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/media/StreamTracks.h:195:12 in GraphRate
Thread T34 (MediaManager) created by T0 (file:// Content) here:
    #0 0x4ac52d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:204:3
    #1 0x7ff9566a092f in CreateThread /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:135:14
    #2 0x7ff9566a092f in PlatformThread::Create(unsigned long, PlatformThread::Delegate*, unsigned long*) /builds/worker/workspace/build/src/ipc/chromium/src/base/platform_thread_posix.cc:146
    #3 0x7ff9566b0f0f in base::Thread::StartWithOptions(base::Thread::Options const&) /builds/worker/workspace/build/src/ipc/chromium/src/base/thread.cc:99:8
    #4 0x7ff95b72a2f8 in mozilla::MediaManager::Get() /builds/worker/workspace/build/src/dom/media/MediaManager.cpp:1863:36
    #5 0x7ff95b603cae in mozilla::dom::MediaDevices::GetUserMedia(mozilla::dom::MediaStreamConstraints const&, mozilla::dom::CallerType, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/media/MediaDevices.cpp:194:9
    #6 0x7ff95922436f in getUserMedia /builds/worker/workspace/build/src/obj-firefox/dom/bindings/MediaDevicesBinding.cpp:185:45
    #7 0x7ff95922436f in mozilla::dom::MediaDevicesBinding::getUserMedia_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::MediaDevices*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/MediaDevicesBinding.cpp:202
    #8 0x7ff95ab7400f in mozilla::dom::GenericPromiseReturningBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3088:13
    #9 0x7ff9616980a4 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #10 0x7ff9616980a4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #11 0x7ff961699102 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #12 0x7ff9624652f2 in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:176:12
    #13 0x7ff962412dbd in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:359:23
    #14 0x7ff9624434f1 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:511:21
    #15 0x7ff962445d84 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:770:12
    #16 0x7ff961698862 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #17 0x7ff961698862 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:455
    #18 0x7ff9616830d6 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #19 0x7ff9616830d6 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #20 0x7ff961669a70 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #21 0x7ff96169b041 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:706:15
    #22 0x7ff96169b7df in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:738:12
    #23 0x7ff9621920f6 in ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::Value*) /builds/worker/workspace/build/src/js/src/jsapi.cpp:4712:12
    #24 0x7ff958c30bb6 in nsJSUtils::ExecutionContext::CompileAndExec(JS::CompileOptions&, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) /builds/worker/workspace/build/src/dom/base/nsJSUtils.cpp:266:8
    #25 0x7ff95cae8ffd in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2233:25
    #26 0x7ff95cae3289 in mozilla::dom::ScriptLoader::ProcessRequest(mozilla::dom::ScriptLoadRequest*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1876:10
    #27 0x7ff95cac891b in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/worker/workspace/build/src/dom/script/ScriptLoader.cpp:1574:10
    #28 0x7ff95cac4b39 in mozilla::dom::ScriptElement::MaybeProcessScript() /builds/worker/workspace/build/src/dom/script/ScriptElement.cpp:147:18
    #29 0x7ff9579ea6a6 in AttemptToExecute /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIScriptElement.h:240:18
    #30 0x7ff9579ea6a6 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:736
    #31 0x7ff9579e3aed in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:540:7
    #32 0x7ff9579efa2b in nsHtml5ExecutorFlusher::Run() /builds/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp:131:20
    #33 0x7ff955865cc0 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:395:25
    #34 0x7ff95588c4fd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #35 0x7ff9558a7fb0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:517:10
    #36 0x7ff95673e44a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #37 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #38 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #39 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #40 0x7ff95cc7718a in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
    #41 0x7ff9613ac41b in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:877:22
    #42 0x7ff956691ff9 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #43 0x7ff956691ff9 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #44 0x7ff956691ff9 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #45 0x7ff9613abe01 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:703:34
    #46 0x4f2dfc in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #47 0x4f2dfc in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
    #48 0x7ff97494582f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291

==16524==ABORTING
Flags: in-testsuite?
[Tracking Requested - why for this release]: fuzz crash
Rank: 13
Priority: -- → P2
I can repro in my Nightly reliably given the attached file. I am on Linux 20180111100722
https://crash-stats.mozilla.com/report/index/3bc9a67b-511e-4d76-a575-51b060180112
mozregression on Linux:

22:49.68 INFO: Last good revision: e3bceb6eb287275e123cb92c487696c6c7325096
22:49.68 INFO: First bad revision: 1fb2d6e0aa2d82c2db246ecd75f7225fecc449ed
22:49.68 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e3bceb6eb287275e123cb92c487696c6c7325096&tochange=1fb2d6e0aa2d82c2db246ecd75f7225fecc449ed
Flags: needinfo?(apehrson)
Thanks, I'll take a look.
Assignee: nobody → apehrson
Flags: needinfo?(apehrson)
We can still take a fix for 59 - I'll track this so we remember to uplift any fixes.
Wow, this is a nice find.

So what happens is this fine order of events:
- The first gUM request comes in. It ends with dispatching a runnable.
- Then the reload() causes an OnNavigation event. Not sure why these happen out of order, but it's irrelevant to the bug. Had these come in order we'd just need different code to trigger the same thing.
- The OnNavigation event clears out some (window id based) state from the first gUM request in MediaManager.
- Then the second gUM request comes in, adds some state for the (same) window id.
- The first gUM request's runnable runs. Sees that the window id based state in MediaManager still exists (it's from the 2nd, alas the wrong, request) and assumes everything is jolly. But when it tries to operate on this state things go south.

I reproed this in a debug build and was hitting an assert rather than the crash mentioned. I imagine the root cause is the same for both modes though.
Is this something you still want to address in 59? We're down to one week of development before the RC builds.
Flags: needinfo?(apehrson)
I don't think 59 is that high prio with this being really rare. If I get it landed in time I could reconsider but for now let's mark it wontfix.
Flags: needinfo?(apehrson)
not tracking based on low crash volume
Status: NEW → ASSIGNED
This has been overtaken by events so the root cause has been fixed. But I still want to add crashtests.
Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

::: dom/media/tests/crashtests/1429507_1.html:7
(Diff revision 1)
> +  <head>
> +    <script>
> +      try { o1 = window.open("") } catch (e) {}
> +      try { o1.location.reload() } catch (e) {}
> +      try { o2 = new RTCPeerConnection({iceServers: [{"url": "stun:d"}]}) } catch (e) {}
> +      try { o1.navigator.mediaDevices.getUserMedia({video: true, fake: true}).catch((error) => {}) } catch (e) {}

Actually, it would be nice if these cleaned up after themselves (track.stop and pc.close()) since leaving these resources open cause nondeterminism in following tests.


I know we track that for mochitests but not crashtests presumably, so maybe this is a lost cause.
Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

> Actually, it would be nice if these cleaned up after themselves (track.stop and pc.close()) since leaving these resources open cause nondeterminism in following tests.
> 
> 
> I know we track that for mochitests but not crashtests presumably, so maybe this is a lost cause.

How do you propose we do that?

With the `reload()` happening the test should already be cleaning itself up. And on navigation to the next test it should be cleaned up by MediaManager anyway.

I fear instead that cleaning up explicitly might affect the test outcome because it's a race between the `reload()` and the second `getUserMedia()` that caused problems in the first place.
Flags: needinfo?(jib)
Comment on attachment 8982547 [details]
Bug 1429507 - Add crashtests.

https://reviewboard.mozilla.org/r/248530/#review254836

> How do you propose we do that?
> 
> With the `reload()` happening the test should already be cleaning itself up. And on navigation to the next test it should be cleaned up by MediaManager anyway.
> 
> I fear instead that cleaning up explicitly might affect the test outcome because it's a race between the `reload()` and the second `getUserMedia()` that caused problems in the first place.

You're right.
Flags: needinfo?(jib)
https://hg.mozilla.org/mozilla-central/rev/d0e20e5a9570
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.