Closed Bug 1544602 Opened 1 year ago Closed 1 year ago

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

Categories

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

Unspecified
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jld, Assigned: bryce)

References

Details

Attachments

(1 file)

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

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

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.

Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a14a4e0b60eb
Make media.gmp.insecure.allow an atomic. r=cpearce
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.