931887 - application crashed [@ mozilla::net::HttpChannelChild::Release() /workers/test/test_csp.html

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Gregor Wagner [:gwagner]]

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]

Comment 1

[Gregor Wagner [:gwagner]]

Maybe a networking bug?

Comment 2

[Josh Matthews [:jdm]]

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

[Andrew Overholt [:overholt]]

(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?

Comment 4

[Honza Bambas (:mayhemer)]

How did you find my name bound to this?

Comment 5

[Andrew Overholt [:overholt]]

(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

[Masatoshi Kimura [:emk]]

Kyle or bent would have much better knowledge than me.

Comment 7

[Andrea Marchesini [:baku]]

Attachment: crash.patch

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.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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

Comment 9

[Andrea Marchesini [:baku]]

Attachment: crash.patch

Bent, what about this?

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 11

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e864fca8c73e

Comment applied.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Fingers crossed!

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e864fca8c73e

Comment 15

[Andrea Marchesini [:baku]]

Attachment: crash.patch

Comment 16

[Andrea Marchesini [:baku]]

This bug is still open.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8365128 [details] [diff] [review]
crash.patch

r=me

Comment 18

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75f2402c0bbf

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75f2402c0bbf

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Can we reenable test_url now that this is fixed?

Comment 21

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=d0a2a5944dea
let's see if it's green. Bug 1014855