1607051 - crash near null in [@ mozilla::dom::BrowsingContext::Get]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c 20200103-5fd4cfacc90d

Test case requires prefs:
dom.allow_scripts_to_close_windows=true
media.autoplay.default=0

docshell/base/BrowsingContext.cpp:109:39: runtime error: member call on null pointer of type 'nsBaseHashtable<nsUint64HashKey, mozilla::dom::BrowsingContext *, mozilla::dom::BrowsingContext *>'
    #0 0x7f433ac30631 in mozilla::dom::BrowsingContext::Get(unsigned long) /home/twsmith/code/mozilla-central/docshell/base/BrowsingContext.cpp:109:39
    #1 0x7f4336217974 in mozilla::dom::ContentMediaController::NotifyMediaStateChanged(mozilla::dom::MediaControlKeysEventListener const*, mozilla::dom::ControlledMediaState) /home/twsmith/code/mozilla-central/dom/media/mediacontrol/ContentMediaController.cpp:100:32
    #2 0x7f433584cee4 in mozilla::dom::HTMLMediaElement::MediaControlEventListener::Stop() dom/html/HTMLMediaElement.cpp:420:5
    #3 0x7f433580d4e7 in mozilla::dom::HTMLMediaElement::~HTMLMediaElement() /home/twsmith/code/mozilla-central/dom/html/HTMLMediaElement.cpp:4260:3
    #4 0x7f43358bbf72 in mozilla::dom::HTMLVideoElement::~HTMLVideoElement() /home/twsmith/code/mozilla-central/dom/html/HTMLVideoElement.cpp:123:39
    #5 0x7f432ee60f40 in SnowWhiteKiller::MaybeKillObject(SnowWhiteKiller::SnowWhiteObject&) xpcom/base/nsCycleCollector.cpp:2429:29
    #6 0x7f432ee56bee in SnowWhiteKiller::~SnowWhiteKiller() /home/twsmith/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:2416:7
    #7 0x7f432ee435fb in nsCycleCollector::FreeSnowWhite(bool) /home/twsmith/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:2609:3
    #8 0x7f432ee48256 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) xpcom/base/nsCycleCollector.cpp:3584:3
    #9 0x7f432ee47c96 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) xpcom/base/nsCycleCollector.cpp:3413:9
    #10 0x7f432ee47934 in nsCycleCollector::ShutdownCollect() /home/twsmith/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:3357:10
    #11 0x7f432ee48c20 in nsCycleCollector::Shutdown(bool) /home/twsmith/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:3645:5
    #12 0x7f432ee4a26c in nsCycleCollector_shutdown(bool) /home/twsmith/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:3960:23
    #13 0x7f432f0d64db in mozilla::ShutdownXPCOM(nsIServiceManager*) /home/twsmith/code/mozilla-central/xpcom/build/XPCOMInit.cpp:710:3
    #14 0x7f433b68137b in ScopedXPCOMStartup::~ScopedXPCOMStartup() /home/twsmith/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:1225:5
    #15 0x7f433b695a2a in mozilla::DefaultDelete<ScopedXPCOMStartup>::operator()(ScopedXPCOMStartup*) const objdir-ff-ubsan/dist/include/mozilla/UniquePtr.h:486:5
    #16 0x7f433b693b54 in mozilla::UniquePtr<ScopedXPCOMStartup, mozilla::DefaultDelete<ScopedXPCOMStartup> >::operator=(std::nullptr_t) objdir-ff-ubsan/dist/include/mozilla/UniquePtr.h:296:5
    #17 0x7f433b68efff in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4764:16
    #18 0x7f433b68faa3 in XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4818:21
    #19 0x55f9fa72cda2 in do_main(int, char**, char**) browser/app/nsBrowserApp.cpp:217:22
    #20 0x55f9fa72c4f0 in main browser/app/nsBrowserApp.cpp:339:16

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/-O109Xh0HcVfkn2Hq1x1sA/index.html

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1607051 - do not access browsing context after XPCOM is shutdown.

After XPCOM is shutdown, sBrowsingContext would be released, so using BrowsingContext::Get() would cause crash. Therefore, after shutdown XPCOM, we should not access browsing context.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a4592adb2fe0
do not access browsing context after XPCOM is shutdown. r=bryce

Comment 4

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/a4592adb2fe0

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Hi Alastor, should we consider uplifting this to Beta? Also, is the testcase something we could land as a crashtest?

Comment 6

[Alastor Wu [:alwu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

Hi Alastor, should we consider uplifting this to Beta? Also, is the testcase something we could land as a crashtest?

As it requires setting pref to specific value, AFAIK, we can't set the pref in the crash test.

Comment 7

[Alastor Wu [:alwu]]

Comment on attachment 9119314 [details]
Bug 1607051 - do not access browsing context after XPCOM is shutdown.

Beta/Release Uplift Approval Request

• User impact if declined: Crash under certain condition
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch prevents us to access browsing context after shutdown XPCOM, and that situation is not common.
• String changes made/needed: no

Comment 8

[Ryan VanderMeulen [:RyanVM]]

You can set prefs in crashtests via the manifest entry. See https://searchfox.org/mozilla-central/source/dom/media/test/crashtests/crashtests.list#135 for example.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1607051 - Add crashtest. r=alwu

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9119314 [details]
Bug 1607051 - do not access browsing context after XPCOM is shutdown.

I'm not having a lot of luck getting a crashtest to work, but that doesn't need to block this landing on Beta in the mean time. Approved for 73.0b4.

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/45cc5093fafe