Closed
Bug 931887
Opened 11 years ago
Closed 11 years ago
application crashed [@ mozilla::net::HttpChannelChild::Release() /workers/test/test_csp.html
Categories
(Core :: DOM: Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: gwagner, Assigned: baku)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Debug Emulator mochitest crash. This prevents us from unhiding debug build tests.
https://tbpl.mozilla.org/php/getParsedLog.php?id=29723283&tree=Pine&full=1#error0
15:29:00 INFO - 4459 INFO TEST-PASS | /tests/dom/workers/test/test_csp.html | Loading data:something
15:29:00 INFO - 4460 INFO TEST-PASS | /tests/dom/workers/test/test_csp.html | Loading data:something
15:29:00 INFO - [Child 711] WARNING: No principal to execute JS with: file ../../../../gecko/dom/src/jsurl/nsJSProtocolHandler.cpp, line 159
15:29:00 INFO - [Child 711] WARNING: No principal to execute JS with: file ../../../../gecko/dom/src/jsurl/nsJSProtocolHandler.cpp, line 159
15:29:00 INFO - [Parent 669] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 205
15:29:00 INFO - [Parent 669] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 205
15:29:00 INFO - [Parent 669] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 205
15:29:00 INFO - [Parent 669] WARNING: RemoveObserver() called for unregistered observer: file ../../gecko/hal/Hal.cpp, line 205
15:29:00 WARNING - PROCESS-CRASH | /tests/dom/workers/test/test_csp.html | application crashed [@ mozilla::net::HttpChannelChild::Release()]
15:29:00 INFO - Crash dump filename: /tmp/tmpJvmB23/0683e9f6-2308-b9b7-1c3432fe-3bca3f88.dmp
15:29:00 INFO - Operating system: Android
15:29:00 INFO - 0.0.0 Linux 2.6.29-00294-g701690d #1 Mon May 20 22:43:07 CST 2013 armv7l Android/full/generic:4.0.4.0.4.0.4/OPENMASTER/eng.cltbld.20131026.155405:eng/test-keys
15:29:00 INFO - CPU: arm
15:29:00 INFO - 0 CPUs
15:29:00 INFO - Crash reason: SIGSEGV
15:29:00 INFO - Crash address: 0x0
15:29:00 INFO - Thread 12 (crashed)
15:29:00 INFO - 0 libxul.so!mozilla::net::HttpChannelChild::Release() [HttpChannelChild.cpp:7ebc9e4f5474 : 64 + 0x4]
15:29:00 INFO - r4 = 0x44bee800 r5 = 0x44bee828 r6 = 0x40222080 r7 = 0x4491fc28
15:29:00 INFO - r8 = 0x45cfe888 r9 = 0x45cfe870 r10 = 0x00000001 fp = 0x00000000
15:29:00 INFO - sp = 0x45cfe818 lr = 0x406dd485 pc = 0x406dd488
15:29:00 INFO - Found by: given as instruction pointer in context
15:29:00 INFO - 1 libxul.so!CacheStorageEvictHelper::~CacheStorageEvictHelper + 0x15
15:29:00 INFO - r4 = 0x45cfe84c r5 = 0x00000000 r6 = 0x45cfe870 r7 = 0x4491fc28
15:29:00 INFO - r8 = 0x45cfe888 r9 = 0x45cfe870 r10 = 0x00000001 fp = 0x00000000
15:29:00 INFO - sp = 0x45cfe830 pc = 0x405c804b
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 2 libxul.so!nsCOMPtr<nsIChannel>::Assert_NoQueryNeeded() [nsCOMPtr.h : 525 + 0x5]
15:29:00 INFO - r4 = 0x45cfe888 r5 = 0x00000000 r6 = 0x45cfe870 r7 = 0x4491fc28
15:29:00 INFO - r8 = 0x45cfe888 r9 = 0x45cfe870 r10 = 0x00000001 fp = 0x00000000
15:29:00 INFO - sp = 0x45cfe838 pc = 0x406417c5
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 3 libxul.so!mozilla::dom::workers::WorkerPrivate::GetLoadInfo(JSContext*, nsPIDOMWindow*, mozilla::dom::workers::WorkerPrivate*, nsAString_internal const&, bool, mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::LoadInfo*) [nsCOMPtr.h : 1327 + 0xb]
15:29:00 INFO - r4 = 0x44943bc0 r5 = 0x4491fc00 r6 = 0x45cfe870 r7 = 0x4491fc28
15:29:00 INFO - r8 = 0x45cfe888 r9 = 0x45cfe870 r10 = 0x00000001 fp = 0x00000000
15:29:00 INFO - sp = 0x45cfe860 pc = 0x40d379e9
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 4 libxul.so!mozilla::dom::workers::WorkerPrivate::Create(JSContext*, JS::Handle<JSObject*>, mozilla::dom::workers::WorkerPrivate*, nsAString_internal const&, bool, bool, nsAString_internal const&, mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::LoadInfo*) [WorkerPrivate.cpp:7ebc9e4f5474 : 3240 + 0x15]
15:29:00 INFO - r4 = 0x45cfe940 r5 = 0x4491fc00 r6 = 0x44545660 r7 = 0x427fa244
15:29:00 INFO - r8 = 0x00000000 r9 = 0x44943bc0 r10 = 0x45cfea24 fp = 0x00000000
15:29:00 INFO - sp = 0x45cfe920 pc = 0x40d384e1
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 5 libxul.so!Worker::ConstructInternal [Worker.cpp:7ebc9e4f5474 : 594 + 0x19]
15:29:00 INFO - r4 = 0x44943bc0 r5 = 0x4491fc00 r6 = 0x44545660 r7 = 0x00000000
15:29:00 INFO - r8 = 0x427fa244 r9 = 0x45cfe9f4 r10 = 0x45cfe9c0 fp = 0x47537040
15:29:00 INFO - sp = 0x45cfe9a0 pc = 0x40d2e399
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 6 libxul.so!Worker::Construct [Worker.cpp:7ebc9e4f5474 : 267 + 0x5]
15:29:00 INFO - r4 = 0x00000000 r5 = 0x44943bc0 r6 = 0x45cfeb14 r7 = 0x40d2e4e9
15:29:00 INFO - r8 = 0x45927078 r9 = 0x45cfeaa8 r10 = 0x45927060 fp = 0xfffff6f4
15:29:00 INFO - sp = 0x45cfea80 pc = 0x40d2e501
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 7 libxul.so!js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [jscntxtinlines.h:7ebc9e4f5474 : 220 + 0x3]
15:29:00 INFO - r4 = 0x45927070 r5 = 0x00000001 r6 = 0x45cfeb14 r7 = 0x40d2e4e9
15:29:00 INFO - r8 = 0x45927078 r9 = 0x45cfeaa8 r10 = 0x45927060 fp = 0xfffff6f4
15:29:00 INFO - sp = 0x45cfeaa0 pc = 0x41ae8d7d
15:29:00 INFO - Found by: call frame info
15:29:00 INFO - 8 libxul.so!js::CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [jscntxtinlines.h:7ebc9e4f5474 : 253 + 0x9]
Reporter | ||
Comment 1•11 years ago
|
||
Maybe a networking bug?
Comment 2•11 years ago
|
||
The CacheStorageEvictHelper::~CacheStorageEvictHelper frame is a red-herring; there's no way that code is being called. We're crashing on the NS_ASSERT_OWNINGTHREAD line in Release, which presumably means that |this| is null. I'm not sure why the check of mRefCnt on the line previous isn't triggering if that's the case, though. However, I have to say that http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#654 looks suspicious to me - if ChannelFromScriptURLMainThread fails, we'll end up releasing the channel object on the worker thread, while we attempt to proxy the release to the main thread in WorkerPrivate::GetLoadInfo.
Comment 3•11 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #2)
> [...] We're crashing on the
> NS_ASSERT_OWNINGTHREAD line in Release, which presumably means that |this|
> is null. I'm not sure why the check of mRefCnt on the line previous isn't
> triggering if that's the case, though. However, I have to say that
> http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.
> cpp#654 looks suspicious to me - if ChannelFromScriptURLMainThread fails,
> we'll end up releasing the channel object on the worker thread, while we
> attempt to proxy the release to the main thread in
> WorkerPrivate::GetLoadInfo.
Masatoshi and Honza, I see your name in related blame output. Any thoughts?
Flags: needinfo?(honzab.moz)
Flags: needinfo?(VYV03354)
Comment 5•11 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #4)
> How did you find my name bound to this?
I don't remember :) Feel free to ignore since looking now I have no idea why I thought you were knowledgeable here.
Comment 6•11 years ago
|
||
Kyle or bent would have much better knowledge than me.
Flags: needinfo?(VYV03354)
Assignee | ||
Comment 7•11 years ago
|
||
I sent this patch to try but emulator builds are broken so I cannot fully test it. If this fix is ok, I'll push to m-i only if it's green on try.
Attachment #8364500 -
Flags: review?(khuey)
Comment on attachment 8364500 [details] [diff] [review]
crash.patch
Review of attachment 8364500 [details] [diff] [review]:
-----------------------------------------------------------------
I don't think this is right. We're not destructing a LoadInfo when we crash. I'm pretty sure the problem is that debug builds call AssertNoQueryNeeded when we assign from already_AddRefed<>, so LoadInfo::StealFrom() is not doing what we want.
http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.h#160
I think this needs to use swap() on each nsCOMPtr (after first asserting that the members are null).
Attachment #8364500 -
Flags: review?(khuey) → review-
Assignee | ||
Comment 9•11 years ago
|
||
Bent, what about this?
Attachment #8364500 -
Attachment is obsolete: true
Attachment #8364524 -
Flags: review?(bent.mozilla)
Comment on attachment 8364524 [details] [diff] [review]
crash.patch
Review of attachment 8364524 [details] [diff] [review]:
-----------------------------------------------------------------
I think this is right.
::: dom/workers/WorkerPrivate.h
@@ +159,5 @@
> void
> StealFrom(LoadInfo& aOther)
> {
> + aOther.mBaseURI.swap(mBaseURI);
> + MOZ_ASSERT(!aOther.mBaseURI);
I would switch all these to assert !mFoo before swapping.
Attachment #8364524 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e864fca8c73e
Comment applied.
Fingers crossed!
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Comment 15•11 years ago
|
||
Attachment #8364524 -
Attachment is obsolete: true
Attachment #8365128 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 16•11 years ago
|
||
This bug is still open.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 17•11 years ago
|
||
Comment on attachment 8365128 [details] [diff] [review]
crash.patch
r=me
Attachment #8365128 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 18•11 years ago
|
||
Comment 19•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Can we reenable test_url now that this is fixed?
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 21•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d0a2a5944dea
let's see if it's green. Bug 1014855
Flags: needinfo?(amarchesini)
You need to log in
before you can comment on or make changes to this bug.
Description
•