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

RESOLVED FIXED in Firefox 68

Status

()

defect
P2
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: jld, Assigned: bryce)

Tracking

unspecified
mozilla68
Unspecified
Linux
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

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>.

Assignee

Comment 1

2 months ago

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.

Assignee: nobody → bvandyk
Priority: -- → P2
Assignee

Comment 2

2 months ago

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 4

2 months ago
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a14a4e0b60eb
Make media.gmp.insecure.allow an atomic. r=cpearce

Comment 5

2 months ago
bugherder
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.