Closed
Bug 1332402
Opened 7 years ago
Closed 7 years ago
Shutdown abort after using BrowserStack, with fatal "Assertion failure: !mThread (SingletonThreads should be Released and shut down before exit!), at media/mtransport/nr_socket_prsock.cpp:181"
Categories
(Core :: WebRTC, defect, P1)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: dholbert, Assigned: bwc)
References
()
Details
(Keywords: assertion)
Attachments
(3 files)
STR: 1. Be using a debug build of mozilla-central. (Mine is disable-optimize enable-debug.) 2. Visit http://browserstack.com/ , log in, and start a remote browser session there (e.g. with Windows 10 & Edge 14). You can create a free trial account if you don't have an account already. (They'll require you to install a restartless browser extension the first time, too.) 3. Click the BrowserStack "Stop" button to end your session. 4. Quit Firefox. ACTUAL RESULTS: Shutdown abort, with this fatal assertion: Assertion failure: !mThread (SingletonThreads should be Released and shut down before exit!), at media/mtransport/nr_socket_prsock.cpp:181 Backtrace: ========== #0 0x00007fecce3e5e02 in mozilla::SingletonThreadHolder::~SingletonThreadHolder() (this=0x7fecab84a700) at $SRC/media/mtransport/nr_socket_prsock.cpp:181 #1 0x00007fecce3e5d4e in mozilla::SingletonThreadHolder::Release() (this=0x7fecab84a700) at $SRC/media/mtransport/nr_socket_prsock.cpp:192 #2 0x00007fecce3e5c86 in mozilla::StaticRefPtr<mozilla::SingletonThreadHolder>::AssignAssumingAddRef(mozilla::SingletonThreadHolder*) (this=0x7fecd7003da8 <mozilla::sThread>, aNewPtr=0x0) at $OBJ/dist/include/mozilla/StaticPtr.h:161 #3 0x00007fecce3e5b8c in mozilla::StaticRefPtr<mozilla::SingletonThreadHolder>::AssignWithAddref(mozilla::SingletonThreadHolder*) (this=0x7fecd7003da8 <mozilla::sThread>, aNewPtr=0x0) at $OBJ/dist/include/mozilla/StaticPtr.h:153 #4 0x00007fecce3e581f in mozilla::StaticRefPtr<mozilla::SingletonThreadHolder>::operator=(mozilla::SingletonThreadHolder*) (this=0x7fecd7003da8 <mozilla::sThread>, aRhs=0x0) at $OBJ/dist/include/mozilla/StaticPtr.h:106 #5 0x00007fecce3e6180 in mozilla::ClearOnShutdown_Internal::PointerClearer<mozilla::StaticRefPtr<mozilla::SingletonThreadHolder> >::Shutdown() (this=0x7fecab96b520) at $OBJ/dist/include/mozilla/ClearOnShutdown.h:81 #6 0x00007feccd0b6251 in mozilla::KillClearOnShutdown(mozilla::ShutdownPhase) (aPhase=mozilla::ShutdownPhase::ShutdownFinal) at $SRC/xpcom/base/ClearOnShutdown.cpp:37 #7 0x00007feccd20939d in mozilla::ShutdownXPCOM(nsIServiceManager*) (aServMgr=0x0) at $SRC/xpcom/build/XPCOMInit.cpp:965 #8 0x00007feccd208fe5 in NS_ShutdownXPCOM(nsIServiceManager*) (aServMgr=0x0) at $SRC/xpcom/build/XPCOMInit.cpp:853 #9 0x00007fecd234dc7c in XRE_TermEmbedding() () at $SRC/toolkit/xre/nsEmbedFunctions.cpp:216 #10 0x00007feccdad7a62 in mozilla::ipc::ScopedXREEmbed::Stop() (this=0x7fece0025998) at $SRC/ipc/glue/ScopedXREEmbed.cpp:117 jesup, looks like this SingletonThreadHolder thing is from bug 1145354. Perhaps you or bwc could take a look?
Reporter | ||
Comment 1•7 years ago
|
||
It looks like we handle this gracefully in opt builds, but it's severe enough that we assert fatally in debug builds: > ~SingletonThreadHolder() > { > r_log(LOG_GENERIC,LOG_DEBUG,"Deleting SingletonThreadHolder"); > MOZ_ASSERT(!mThread, "SingletonThreads should be Released and shut down before exit!"); > if (mThread) { > mThread->Shutdown(); > mThread = nullptr; > } > } http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/media/mtransport/nr_socket_prsock.cpp#175,181
Keywords: assertion
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(rjesup)
Assignee | ||
Comment 2•7 years ago
|
||
Looking.
Reporter | ||
Updated•7 years ago
|
Summary: Shutdown abort after using BrowserStack, with "Assertion failure: !mThread (SingletonThreads should be Released and shut down before exit!), at media/mtransport/nr_socket_prsock.cpp:181" → Shutdown abort after using BrowserStack, with fatal "Assertion failure: !mThread (SingletonThreads should be Released and shut down before exit!), at media/mtransport/nr_socket_prsock.cpp:181"
Assignee | ||
Comment 3•7 years ago
|
||
So I guess on shutdown the usual calls to ReleaseUse aren't happening because STS or the IO thread isn't running anymore? I wonder if we can call ReleaseUse sooner, maybe even within ~NrUdpSocketIpc?
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
We don't *need* to assert on this; just complain. Most likely a transport/etc is stuck in some GC wait or CC loop - which is a more general thing we should try to track down and avoid, but not on this bug. Also, probably makes sense to clear the thread at shutdown-threads time, not afterwards.
Attachment #8828483 -
Flags: review?(docfaraday)
Updated•7 years ago
|
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8828482 [details] Bug 1332402 - Part 1: Simplify shutdown of the IO thread. https://reviewboard.mozilla.org/r/105866/#review106802 I think this is ok, since I believe the io_thread_'s event queue will get all processed and emptied. We probably want this, and my patch. (we only need my patch I think; this does simplify things a bit if all our assumptions are right.) ::: media/mtransport/nr_socket_prsock.cpp:1136 (Diff revision 1) > RUN_ON_THREAD(io_thread_, > mozilla::WrapRunnableNM(&NrUdpSocketIpc::release_child_i, > - socket_child_.forget().take(), > + socket_child_.forget().take()), > - sts_thread_), > NS_DISPATCH_NORMAL); > + sThread->ReleaseUse(); At minimum, this needs a comment that ReleaseUse may shut down io_thread_ after it processes the WrapRunnable. Since Shutdown() spins the event loop until the shutdown message is processed, this should work.
Attachment #8828482 -
Flags: review+
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8828482 [details] Bug 1332402 - Part 1: Simplify shutdown of the IO thread. https://reviewboard.mozilla.org/r/105866/#review106802 > At minimum, this needs a comment that ReleaseUse may shut down io_thread_ after it processes the WrapRunnable. Since Shutdown() spins the event loop until the shutdown message is processed, this should work. I can add a comment, sure.
Updated•7 years ago
|
Rank: 15
Priority: -- → P1
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8828483 -
Flags: review?(docfaraday) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8828928 [details] Bug 1332402 - Part 2: just complain if SingletonThread is still active at shutdown https://reviewboard.mozilla.org/r/106146/#review107148
Attachment #8828928 -
Flags: review?(docfaraday) → review+
Assignee | ||
Updated•7 years ago
|
Assignee: rjesup → docfaraday
Comment 12•7 years ago
|
||
Pushed by bcampen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/94ed8555d2ee Part 1: Simplify shutdown of the IO thread. r=jesup https://hg.mozilla.org/integration/autoland/rev/c1e8aed2515e Part 2: just complain if SingletonThread is still active at shutdown r=bwc
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/94ed8555d2ee https://hg.mozilla.org/mozilla-central/rev/c1e8aed2515e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 14•7 years ago
|
||
Is this worth backporting to 53?
status-firefox52:
--- → wontfix
status-firefox-esr52:
--- → wontfix
Flags: needinfo?(rjesup) → needinfo?(docfaraday)
Assignee | ||
Comment 15•7 years ago
|
||
For an assertion on shutdown in rare cases? Nah.
Flags: needinfo?(docfaraday)
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•