1672876 - crash near null in [@ mozilla::dom::Document::UnregisterActivityObserver]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Tyson Smith [:tsmith]]

I don't have a good test case but I will attach a Pernosco session.

==1986169==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000288 (pc 0x7fdc9df4c9a1 bp 0x7ffd142f8780 sp 0x7ffd142f8770 T0)
==1986169==The signal is caused by a READ memory access.
==1986169==Hint: address points to the zero page.
    #0 0x7fdc9df4c9a1 in get /builds/worker/workspace/obj-build/dist/include/mozilla/UniquePtr.h:287:32
    #1 0x7fdc9df4c9a1 in operator bool /builds/worker/workspace/obj-build/dist/include/mozilla/UniquePtr.h:285:43
    #2 0x7fdc9df4c9a1 in mozilla::dom::Document::UnregisterActivityObserver(nsISupports*) /gecko/dom/base/Document.cpp:12068:8
    #3 0x7fdca108a19e in mozilla::dom::MediaSession::Shutdown() /gecko/dom/media/mediasession/MediaSession.cpp:62:9
    #4 0x7fdc9e0498c5 in mozilla::dom::Navigator::Invalidate() /gecko/dom/base/Navigator.cpp:236:20
    #5 0x7fdc9e049e10 in mozilla::dom::Navigator::cycleCollection::Unlink(void*) /gecko/dom/base/Navigator.cpp:135:8
    #6 0x7fdc9a695f65 in nsCycleCollector::CollectWhite() /gecko/xpcom/base/nsCycleCollector.cpp:3083:26
    #7 0x7fdc9a698b1d in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /gecko/xpcom/base/nsCycleCollector.cpp:3432:24
    #8 0x7fdc9a69bf1e in nsCycleCollector_collectSlice(js::SliceBudget&, bool) /gecko/xpcom/base/nsCycleCollector.cpp:3920:21
    #9 0x7fdc9e22c4ce in nsJSContext::RunCycleCollectorSlice(mozilla::TimeStamp) /gecko/dom/base/nsJSEnvironment.cpp:1590:3
    #10 0x7fdc9e22ef9f in CCRunnerFired(mozilla::TimeStamp) /gecko/dom/base/nsJSEnvironment.cpp:1932:7
    #11 0x7fdc9a8157a2 in operator() /builds/worker/fetches/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/7.4.0/../../../../include/c++/7.4.0/bits/std_function.h:706:14
    #12 0x7fdc9a8157a2 in mozilla::IdleTaskRunner::Run() /gecko/xpcom/threads/IdleTaskRunner.cpp:54:14
    #13 0x7fdc9a8797b9 in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:244:16
    #14 0x7fdc9a838653 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:514:26
    #15 0x7fdc9a8361cd in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:396:15
    #16 0x7fdc9a83648d in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:170:36
    #17 0x7fdc9a887351 in operator() /gecko/xpcom/threads/TaskController.cpp:84:37
    #18 0x7fdc9a887351 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #19 0x7fdc9a85ba63 in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1234:14
    #20 0x7fdc9a865b5c in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #21 0x7fdc9bb309cf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:87:21
    #22 0x7fdc9ba34f41 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #23 0x7fdc9ba34f41 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #24 0x7fdc9ba34f41 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #25 0x7fdca27b7147 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:137:27
    #26 0x7fdca64c783f in XRE_RunAppShell() /gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #27 0x7fdc9ba34f41 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #28 0x7fdc9ba34f41 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #29 0x7fdc9ba34f41 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #30 0x7fdca64c6ddc in XRE_InitChildProcess(int, char**, XREChildData const*) /gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #31 0x560dbe6ef08d in content_process_main(mozilla::Bootstrap*, int, char**) /gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #32 0x560dbe6ef4c7 in main /gecko/browser/app/nsBrowserApp.cpp:304:18

Comment 1

[Tyson Smith [:tsmith]]

Actually maybe here is a better place to start.

Comment 2

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/2qK8RZPtuvlMHCKOcsa7eQ/index.html

Comment 3

[Andrew McCreight [:mccr8]]

From looking at the stack, and the fact that it is a null deref, maybe this is a missing null check on mDoc in MediaSession::Shutdown() ? I think that if we Unlink the MediaSession before the Navigator, this can happen. Possibly this is a regression from bug 1665527?

Comment 4

[Andrew McCreight [:mccr8]]

Interestingly, eons ago Jesse Ruderman proposed adding a flag to the cycle collector that would make it unlink objects in reverse order to help with fuzzing, and I think this is a case (maybe the first case I've seen) where I think it would help get a better test case.

Comment 5

[Andrew McCreight [:mccr8]]

(I think the MediaSession object is like a helper object for the Navigator, and the Navigator hangs right off of a document or whatever, so I think in most cases we'll traverse (and thus unlink) the Navigator before the MediaSession, which might be why this is so hard to reproduce.)

Comment 6

[Alastor Wu [:alwu]]

Thank for catching that! will submit a patch.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1672876 - run shutdown during unlink.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05884654e7f2
run shutdown during unlink. r=mccr8

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/05884654e7f2

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:alwu, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Alastor Wu [:alwu]]

This is really hard to reproduce, so I won't make an uplift request for that.