Closed Bug 997286 Opened 11 years ago Closed 11 years ago

NS_NewNamedThread can't deal with suicidal events

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla31
Tracking Flags:
Tracking Status
firefox29 --- unaffected
firefox30 --- unaffected
firefox31 --- fixed
firefox-esr24 --- unaffected

People

(Reporter: RyanVM, Assigned: jesup)

References

Details

(Keywords: crash, intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Make NS_NewNamedThread safe if the event tries to commit suicide
2.59 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
Don't depend on NewThread event timing; test suicidal events in NS_New(Named)Thread()
3.74 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=37938183&tree=Mozilla-Inbound b2g_emulator_vm mozilla-inbound debug test mochitest-debug-10 on 2014-04-16 09:42:13 PDT for push 7cfba1345851 slave: tst-linux64-spot-906 09:55:35 INFO - 0 INFO Running tests 1235-1371/2056 09:55:35 INFO - 1 INFO SimpleTest START 09:55:36 INFO - 2 INFO TEST-START | /tests/dom/media/tests/mochitest/test_getUserMedia_constraints.html 09:55:36 INFO - [Child 717] WARNING: TabChild::SetFocus not supported in TabChild: file ../../../gecko/dom/ipc/TabChild.cpp, line 1120 09:55:37 INFO - [Parent 657] WARNING: No docshells for remote frames!: file ../../../../gecko/content/base/src/nsFrameLoader.cpp, line 627 09:55:37 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:40 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:42 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:43 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:46 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:53 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:53 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:54 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:55 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:55 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:55:56 INFO - 3 INFO TEST-INFO | MEMORY STAT vsize after test: 101134336 09:55:56 INFO - 4 INFO TEST-INFO | MEMORY STAT vsizeMaxContiguous not supported in this build configuration. 09:55:56 INFO - 5 INFO TEST-INFO | MEMORY STAT residentFast after test: 46768128 09:55:56 INFO - 6 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 12923736 09:55:57 INFO - 7 INFO TEST-END | /tests/dom/media/tests/mochitest/test_getUserMedia_constraints.html | finished in 20821ms 09:55:58 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:01 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:03 INFO - 8 INFO TEST-START | /tests/dom/media/tests/mochitest/test_getUserMedia_exceptions.html 09:56:06 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:06 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:12 INFO - [Parent 657] WARNING: A control runnable was posted to a worker that is already shutting down!: file ../../../gecko/dom/workers/WorkerPrivate.cpp, line 2238 09:56:12 INFO - [Parent 657] WARNING: A control runnable was posted to a worker that is already shutting down!: file ../../../gecko/dom/workers/WorkerPrivate.cpp, line 2238 09:56:12 INFO - [Parent 657] WARNING: A control runnable was posted to a worker that is already shutting down!: file ../../../gecko/dom/workers/WorkerPrivate.cpp, line 2238 09:56:12 INFO - [Parent 657] WARNING: A control runnable was posted to a worker that is already shutting down!: file ../../../gecko/dom/workers/WorkerPrivate.cpp, line 2238 09:56:13 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:14 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:15 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:15 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:16 INFO - 9 INFO TEST-INFO | MEMORY STAT vsize after test: 99102720 09:56:16 INFO - 10 INFO TEST-INFO | MEMORY STAT residentFast after test: 46465024 09:56:16 INFO - 11 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 12258904 09:56:16 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:16 INFO - 12 INFO TEST-END | /tests/dom/media/tests/mochitest/test_getUserMedia_exceptions.html | finished in 13355ms 09:56:17 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:17 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:17 INFO - 13 INFO TEST-START | /tests/dom/media/tests/mochitest/test_getUserMedia_gumWithinGum.html 09:56:18 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:18 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:19 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:19 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:20 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:20 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:21 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:21 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:27 INFO - 14 INFO TEST-INFO | MEMORY STAT vsize after test: 103362560 09:56:27 INFO - 15 INFO TEST-INFO | MEMORY STAT residentFast after test: 47906816 09:56:27 INFO - 16 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 13456428 09:56:28 INFO - 17 INFO TEST-END | /tests/dom/media/tests/mochitest/test_getUserMedia_gumWithinGum.html | finished in 10649ms 09:56:29 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:29 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:33 INFO - 18 INFO TEST-START | /tests/dom/media/tests/mochitest/test_getUserMedia_playAudioTwice.html 09:56:33 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:34 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:36 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:42 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:42 INFO - [Child 717] WARNING: Transparent content with displayports can be expensive.: file ../../../gecko/layout/base/nsDisplayList.cpp, line 1338 09:56:44 INFO - ############ ErrorPage.js 09:56:44 INFO - [Parent 657] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 204 09:56:44 INFO - [Parent 657] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 204 09:56:44 INFO - [Parent 657] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 204 09:56:44 INFO - [Parent 657] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 204 09:56:44 INFO - [Parent 657] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 204 09:56:45 INFO - [Parent 657] WARNING: NS_ENSURE_TRUE(mCallback) failed: file ../../../../gecko/content/base/src/nsFrameMessageManager.cpp, line 640 09:56:45 INFO - ************************************************************ 09:56:45 INFO - * Call to xpconnect wrapped JSObject produced this error: * 09:56:45 INFO - [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIMessageSender.sendAsyncMessage]" nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)" location: "JS frame :: chrome://specialpowers/content/SpecialPowersObserver.js :: SpecialPowersObserver.prototype._sendAsyncMessage :: line 96" data: no] 09:56:45 INFO - ************************************************************ 10:04:15 INFO - B2GRunner TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_getUserMedia_playAudioTwice.html | application timed out after 450.0 seconds with no output 10:04:15 INFO - B2GRunner INFO | checking for crashes in '/data/local/tests/profile/minidumps' 10:04:32 WARNING - PROCESS-CRASH | /tests/dom/media/tests/mochitest/test_getUserMedia_playAudioTwice.html | application crashed [@ mozilla::BlockingResourceBase::CheckAcquire(mozilla::CallStack const&)] 10:04:32 INFO - Crash dump filename: /tmp/tmpoSNEVl/633643b9-19d9-237d-4d1f9383-69df62f3.dmp 10:04:32 INFO - Operating system: Android 10:04:32 INFO - 0.0.0 Linux 2.6.29-00302-g586075d #31 Mon Feb 24 10:28:23 PST 2014 armv7l Android/full/generic:4.0.4.0.4.0.4/OPENMASTER/eng.cltbld.20140416.115048:eng/test-keys 10:04:32 INFO - CPU: arm 10:04:32 INFO - 0 CPUs 10:04:32 INFO - Crash reason: SIGSEGV 10:04:32 INFO - Crash address: 0x5a5a5a5e 10:04:32 INFO - Thread 14 (crashed) 10:04:32 INFO - 0 libxul.so!mozilla::BlockingResourceBase::CheckAcquire(mozilla::CallStack const&) [BlockingResourceBase.cpp:7cfba1345851 : 99 + 0x0] 10:04:32 INFO - r4 = 0x44fbc890 r5 = 0x44fbc890 r6 = 0x448711f0 r7 = 0x44bffabc 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x024a4432 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffa08 lr = 0x405ca3ab pc = 0x405c9fb0 10:04:32 INFO - Found by: given as instruction pointer in context 10:04:32 INFO - 1 libxul.so!mozilla::OffTheBooksMutex::Lock() [BlockingResourceBase.cpp:7cfba1345851 : 229 + 0x5] 10:04:32 INFO - r4 = 0x44bffabc r5 = 0x44fbc890 r6 = 0x448711f0 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x00000000 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffab8 pc = 0x405ca3ab 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 2 libxul.so!mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock [Mutex.h : 173 + 0x5] 10:04:32 INFO - r4 = 0x44bffae4 r5 = 0x44bffb14 r6 = 0x448711f0 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x00000000 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffad0 pc = 0x405c3181 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 3 libxul.so!nsThread::GetObserver(nsIThreadObserver**) [nsThread.cpp:7cfba1345851 : 776 + 0x3] 10:04:32 INFO - r4 = 0x44fbc880 r5 = 0x44bffb14 r6 = 0x448711f0 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x00000000 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffae0 pc = 0x4061fe41 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 4 libxul.so!nsThread::PutEvent(nsIRunnable*, nsThread::nsNestedEventTarget*) [nsThread.h:7cfba1345851 : 76 + 0x7] 10:04:32 INFO - r4 = 0x44fbc880 r5 = 0x00000000 r6 = 0x448711f0 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x00000000 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffb00 pc = 0x4061fecb 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 5 libxul.so!nsThread::DispatchInternal(nsIRunnable*, unsigned int, nsThread::nsNestedEventTarget*) [nsThread.cpp:7cfba1345851 : 472 + 0x9] 10:04:32 INFO - r4 = 0x00000000 r5 = 0x448711f0 r6 = 0x42a6e3e8 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x448711f0 r9 = 0x00000000 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffb30 pc = 0x40620aa7 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 6 libxul.so!nsThread::Dispatch(nsIRunnable*, unsigned int) [nsThread.cpp:7cfba1345851 : 483 + 0xb] 10:04:32 INFO - r4 = 0x00000000 r5 = 0x448711f0 r6 = 0x44fbc880 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x40255084 r9 = 0x00000001 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffb68 pc = 0x40620b5d 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 7 libxul.so!mozilla::AudioStream::Init(int, int, mozilla::dom::AudioChannel, mozilla::AudioStream::LatencyRequest) [nsThreadUtils.h : 78 + 0xb] 10:04:32 INFO - r4 = 0x44871214 r5 = 0x44bffc24 r6 = 0x448711f0 r7 = 0x44fbc880 10:04:32 INFO - r8 = 0x40255084 r9 = 0x00000001 r10 = 0x00000000 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffb78 pc = 0x412dde25 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 8 libxul.so!mozilla::MediaStreamGraphImpl::CreateOrDestroyAudioStreams(long long, mozilla::MediaStream*) [MediaStreamGraph.cpp:7cfba1345851 : 846 + 0x13] 10:04:32 INFO - r4 = 0x4021d7c0 r5 = 0x44499918 r6 = 0x40255080 r7 = 0x44ec1690 10:04:32 INFO - r8 = 0x4021d8bc r9 = 0x00000001 r10 = 0x4230a442 fp = 0x00000000 10:04:32 INFO - sp = 0x44bffc50 pc = 0x41300d4d 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 9 libxul.so!mozilla::MediaStreamGraphImpl::RunThread() [MediaStreamGraph.cpp:7cfba1345851 : 1344 + 0xb] 10:04:32 INFO - r4 = 0x44ec1690 r5 = 0x4021d7c0 r6 = 0x00000005 r7 = 0x00000000 10:04:32 INFO - r8 = 0x4234fd73 r9 = 0x4234fd50 r10 = 0x4234eba2 fp = 0x0000cccc 10:04:32 INFO - sp = 0x44bffcc8 pc = 0x413027e7 10:04:32 INFO - Found by: call frame info 10:04:32 INFO - 10 libxul.so!mozilla::::MediaStreamGraphInitThreadRunnable::Run [MediaStreamGraph.cpp:7cfba1345851 : 1485 + 0x5] 10:04:32 INFO - r4 = 0x44cf6b40 r5 = 0x00000000 r6 = 0x44bffdd4 r7 = 0x44bffdd0 10:04:32 INFO - r8 = 0x00000000 r9 = 0x44bffe0f r10 = 0x44fa0ebc fp = 0x00000000 10:04:32 INFO - sp = 0x44bffd90 pc = 0x41302b0f 10:04:32 INFO - Found by: call frame info
Should match NS_NewThread() now.
Attachment #8407775 - Attachment is obsolete: true
Attachment #8407783 - Flags: review?(benjamin)
Attachment #8407783 - Flags: review?(benjamin) → review+
Blocks: 996487
Component: Video/Audio → XPCOM
Summary: Intermittent test_getUserMedia_playAudioTwice.html | application timed out after 450.0 seconds with no output | application crashed [@ mozilla::BlockingResourceBase::CheckAcquire(mozilla::CallStack const&)] → NS_NewNamedThread can't deal with suicidal events
Blocks: 919215
No longer depends on: 919215
This is one of the awkward features of reference counting systems where objects can live with no references, but the changes to NS_NewNamedThread() didn't change any reference counting.
https://hg.mozilla.org/mozilla-central/rev/ed17d9bbe15c
Assignee: nobody → rjesup
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Reopen to land a test and to clean up the usage (the mThread output of NS_NewNamedThread isn't guaranteed to be set when the event executes). See also bug 988881.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Split AudioInitTask fix out from bug 988881 and added a CPP unit test for suicidal events
Comment on attachment 8408011 [details] [diff] [review] Don't depend on NewThread event timing; test suicidal events in NS_New(Named)Thread() The NS_NewNamedThread() patch was right to do, but didn't actually plug the hole here, just made it a null deref instead of a UAF. Fixed it by not using a suicide event (could have used mThread = goGetCurrentThread() inside Run instead with a suicide event; both ways work). Also added a CPP unit test for event suicide.
Attachment #8408011 - Flags: review?(benjamin)
Attachment #8408011 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/2c59614ef6d3 https://hg.mozilla.org/mozilla-central/rev/12d543cfa167
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
OS: Gonk (Firefox OS) → All
Hardware: ARM → All
Not entirely sure unaffected is really the right status for older branches, but whatever. Feel free to request approval for branch uplift if it makes sense to do so.
status-firefox29: --- → unaffected
status-firefox30: --- → unaffected
status-firefox31: --- → fixed
status-firefox-esr24: --- → unaffected
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: