1544602 - Assertion failure: IsAtomic<bool>::value || NS_IsMainThread() (Non-atomic static pref 'media.gmp.insecure.allow' being accessed on background thread)

Status: RESOLVED FIXED

(Core :: Audio/Video: GMP, defect, P2)

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

I saw this during startup while testing something else (RDD-related, using this AV1 demo page, so I don't know why it would be touching GMP):

Assertion failure: IsAtomic<bool>::value || NS_IsMainThread() (Non-atomic static pref 'media.gmp.insecure.allow' being accessed on background thread), at /home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/mozilla/StaticPrefList.h:1357
#01: mozilla::gmp::CreateGMPParent(mozilla::AbstractThread*) (/home/jld/src/gecko-dev/dom/media/gmp/GMPServiceParent.cpp:776)
#02: mozilla::gmp::GeckoMediaPluginServiceParent::AddOnGMPThread(nsTString<char16_t>) (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:293)
#03: nsTSubstring<char16_t>::~nsTSubstring() (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/nsTSubstring.h:326)
#04: mozilla::detail::ProxyRunnable<mozilla::MozPromise<bool, nsresult, true>, RefPtr<mozilla::MozPromise<bool, nsresult, true> > (mozilla::gmp::GeckoMediaPluginServiceParent::*)(nsTString<char16_t>), mozilla::gmp::GeckoMediaPluginServiceParent, StoreCopyPassByRRef<nsTString<char16_t> > >::Run() (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/nsAutoPtr.h:34)
#05: mozilla::EventTargetWrapper::Runner::Run() (/home/jld/src/gecko-dev/xpcom/threads/AbstractThread.cpp:113)
#06: nsThread::ProcessNextEvent(bool, bool*) (/home/jld/src/gecko-dev/xpcom/threads/nsThread.cpp:1167)
#07: NS_ProcessNextEvent(nsIThread*, bool) (/home/jld/src/gecko-dev/xpcom/threads/nsThreadUtils.cpp:486)
#08: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (/home/jld/src/gecko-dev/ipc/glue/MessagePump.cpp:303)
#09: MessageLoop::RunInternal() (/home/jld/src/gecko-dev/ipc/chromium/src/base/message_loop.cc:315)
#10: MessageLoop::AutoRunState::~AutoRunState() (/home/jld/src/gecko-dev/ipc/chromium/src/base/message_loop.cc:584)
#11: nsThread::ThreadFunc(void*) (/home/jld/src/gecko-dev/xpcom/threads/nsThread.cpp:456)
#12: _pt_root (/home/jld/src/gecko-dev/nsprpub/pr/src/pthreads/ptthread.c:204)
#13: start_thread (/build/glibc-d2N3Ld/glibc-2.28/nptl/pthread_create.c:487 (discriminator 6))
#14: __GI___clone (/build/glibc-d2N3Ld/glibc-2.28/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:97)
#15: ??? (???:???)

Racing to access a bool that's rarely changed probably isn't security-sensitive, but this seems like something that ought to be fixed, and if I understand correctly it should be fixable by changing the bool to Atomic<bool>.

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

As mentioned in the description, looks like we should make this atomic. We have prior art in bug 1465852. I'll create a patch to fix this.

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1544602 - Make media.gmp.insecure.allow an atomic. r?cpearce

Since this pref can be accessed from the GMP thread (per the bug), it should
be an atomic.

See bug 1465852 for the code the added the assertion hit in this bug, as well as
further context as to why this should be an atomic.

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94fed432eddd6f3eaa3586da3c169c4ef44cd33f

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a14a4e0b60eb
Make media.gmp.insecure.allow an atomic. r=cpearce

Comment 5

[Andreea Pavel [:apavel]]

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