Closed Bug 1374922 Opened 6 years ago Closed 6 years ago

Intermittent TestServerQuitApplication.test_attempt_quit | application crashed [@ RefPtr<mozilla::dom::FetchBody<mozilla::dom::Response> >::operator->]


(Core :: DOM: Core & HTML, defect)

Not set



Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed


(Reporter: intermittent-bug-filer, Assigned: baku)



(Keywords: crash, intermittent-failure, regression)

Crash Data


(4 files, 1 obsolete file)

Crash details:

[task 2017-06-21T00:14:24.695488Z] 00:14:24     INFO - Crash reason:  SIGSEGV
[task 2017-06-21T00:14:24.696501Z] 00:14:24     INFO - Crash address: 0x0

First 10 frames from the stack of the crashing thread 0:

[task 2017-06-21T00:14:24.699673Z] 00:14:24     INFO - Thread 0 (crashed)
[task 2017-06-21T00:14:24.700746Z] 00:14:24     INFO -  0!RefPtr<mozilla::dom::FetchBody<mozilla::dom::Response> >::operator-> [RefPtr.h:86f86f971014 : 314 + 0x1f]
[task 2017-06-21T00:14:24.701851Z] 00:14:24     INFO -     eip = 0xf14d6b9a   esp = 0xffe2e6c0   ebp = 0xffe2e6d8   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.702855Z] 00:14:24     INFO -     esi = 0xffe2e71c   edi = 0xffe2e70c   eax = 0x00000000   ecx = 0xf759a864
[task 2017-06-21T00:14:24.703854Z] 00:14:24     INFO -     edx = 0x00000000   efl = 0x00010282
[task 2017-06-21T00:14:24.704847Z] 00:14:24     INFO -     Found by: given as instruction pointer in context
[task 2017-06-21T00:14:24.705904Z] 00:14:24     INFO -  1!mozilla::dom::FetchBodyConsumer<mozilla::dom::Response>::BeginConsumeBodyMainThread [FetchConsumer.cpp:86f86f971014 : 427 + 0x19]
[task 2017-06-21T00:14:24.706948Z] 00:14:24     INFO -     eip = 0xf14d876f   esp = 0xffe2e6e0   ebp = 0xffe2e738   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.707989Z] 00:14:24     INFO -     esi = 0xffe2e71c   edi = 0xffe2e70c
[task 2017-06-21T00:14:24.709029Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.710132Z] 00:14:24     INFO -  2!BeginConsumeBodyRunnable<mozilla::dom::Response>::Run [FetchConsumer.cpp:86f86f971014 : 67 + 0x14]
[task 2017-06-21T00:14:24.711175Z] 00:14:24     INFO -     eip = 0xf14d8bc4   esp = 0xffe2e740   ebp = 0xffe2e758   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.712226Z] 00:14:24     INFO -     esi = 0xf71d6290   edi = 0xffe2e7c4
[task 2017-06-21T00:14:24.713262Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.714326Z] 00:14:24     INFO -  3!nsThread::ProcessNextEvent [nsThread.cpp:86f86f971014 : 1421 + 0x17]
[task 2017-06-21T00:14:24.715370Z] 00:14:24     INFO -     eip = 0xf0081b89   esp = 0xffe2e760   ebp = 0xffe2e838   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.716414Z] 00:14:24     INFO -     esi = 0xf71d6290   edi = 0xffe2e7c4
[task 2017-06-21T00:14:24.717448Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.718495Z] 00:14:24     INFO -  4!NS_ProcessPendingEvents [nsThreadUtils.cpp:86f86f971014 : 427 + 0xa]
[task 2017-06-21T00:14:24.719517Z] 00:14:24     INFO -     eip = 0xf007d5e5   esp = 0xffe2e840   ebp = 0xffe2e888   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.720503Z] 00:14:24     INFO -     esi = 0xf71d6290   edi = 0xffe2e86f
[task 2017-06-21T00:14:24.721496Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.722514Z] 00:14:24     INFO -  5!mozilla::ShutdownXPCOM [XPCOMInit.cpp:86f86f971014 : 886 + 0xc]
[task 2017-06-21T00:14:24.723518Z] 00:14:24     INFO -     eip = 0xf0098043   esp = 0xffe2e890   ebp = 0xffe2e8e8   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.724510Z] 00:14:24     INFO -     esi = 0xffe2e8c8   edi = 0xffe2e8c0
[task 2017-06-21T00:14:24.725487Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.726520Z] 00:14:24     INFO -  6!ScopedXPCOMStartup::~ScopedXPCOMStartup [nsAppRunner.cpp:86f86f971014 : 1467 + 0x8]
[task 2017-06-21T00:14:24.727519Z] 00:14:24     INFO -     eip = 0xf2c639ec   esp = 0xffe2e8f0   ebp = 0xffe2e928   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.728490Z] 00:14:24     INFO -     esi = 0xffe2e90c   edi = 0xf715f344
[task 2017-06-21T00:14:24.729478Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.730490Z] 00:14:24     INFO -  7!mozilla::DefaultDelete<ScopedXPCOMStartup>::operator() [UniquePtr.h:86f86f971014 : 528 + 0x9]
[task 2017-06-21T00:14:24.731497Z] 00:14:24     INFO -     eip = 0xf2c63a25   esp = 0xffe2e930   ebp = 0xffe2e948   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.732473Z] 00:14:24     INFO -     esi = 0xf715f344   edi = 0xffe2e978
[task 2017-06-21T00:14:24.733438Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.734436Z] 00:14:24     INFO -  8!XREMain::XRE_main [UniquePtr.h:86f86f971014 : 343 + 0x5]
[task 2017-06-21T00:14:24.735434Z] 00:14:24     INFO -     eip = 0xf2c6acd4   esp = 0xffe2e950   ebp = 0xffe2e9a8   ebx = 0xf563ba04
[task 2017-06-21T00:14:24.736415Z] 00:14:24     INFO -     esi = 0x00000000   edi = 0xffe2e978
[task 2017-06-21T00:14:24.737386Z] 00:14:24     INFO -     Found by: call frame info
[task 2017-06-21T00:14:24.738371Z] 00:14:24     INFO -  9!XRE_main [nsAppRunner.cpp:86f86f971014 : 4869 + 0x6]
[task 2017-06-21T00:14:24.739351Z] 00:14:24     INFO -     eip = 0xf2c6af48   esp = 0xffe2e9b0   ebp = 0xffe2eb08   ebx = 0x08070230
[task 2017-06-21T00:14:24.740339Z] 00:14:24     INFO -     esi = 0xffe2e9e4   edi = 0xffe2fc44
[task 2017-06-21T00:14:24.741282Z] 00:14:24     INFO -     Found by: call frame info

Andrea, could this be a regression based on your landing of bug 1373555?
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
What I see here is that FetchConsumer does something after XPCOM shutdown.
On main-thread this should not happen, because FetchConsumer should not survive after that its window goes away.
Attachment #8879867 - Flags: review?(bkelly)
Attached patch part 2 - workersSplinter Review
We should also listen for xpcom-shutdown in workers and main-thread.
Attachment #8879869 - Flags: review?(bkelly)
Component: Marionette → DOM
Product: Testing → Core
Version: Version 3 → unspecified
Attachment #8879867 - Flags: review?(bkelly) → review+
Comment on attachment 8879869 [details] [diff] [review]
part 2 - workers

Review of attachment 8879869 [details] [diff] [review]:

::: dom/fetch/FetchConsumer.cpp
@@ +376,3 @@
>      }
> +  } else {
> +    // If in workers, we need to remove the observer on main-thread.

Why do we need this on workers?  Won't we get notified through the WorkerHolder mechanism?
Attachment #8879869 - Flags: review?(bkelly) → review+
Pushed by
FetchConsumer should not continue if the window is destroyed, r=bkelly
Let's land just the first patch. In case we have the same issue in workers, I land the second patch.
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Andrea, this seems to be still a problem it recent tests. Can you please have a look? Maybe it's related to headless?
Flags: needinfo?(amarchesini)
I just landed a patch today. This is be fixed by now in m-i. Can you please NI again in case it is not fixed in a couple of days?
Flags: needinfo?(amarchesini)
Sure. I will keep an eye on it.
I'm seeing similar-looking issues on Beta/ESR52 since bug 1371889 was uplifted to them. Do we need this patch on those branches as well now?
Flags: needinfo?(amarchesini)
Yes, the patch has to be uplifted, but on beta/esr52 we don't have bug 1373555. We do want to uplift also bug 1373555? This seems a lot of code. Maybe I can provide a patch for beta/esr52 without bringing all the rest.
Flags: needinfo?(amarchesini)
Attached patch m-b (obsolete) — Splinter Review
Attached patch m-bSplinter Review
Patch rebased
Attachment #8882351 - Attachment is obsolete: true
Attached patch m-esr52Splinter Review
Did you intend to request approval on these :)? Would be great to get them in before Monday's Beta7 build (we built Beta6 yesterday on a revision prior to bug 1371889 landing due to concerns that this would turn into a topcrash if it was included).
Flags: needinfo?(amarchesini)
Comment on attachment 8882397 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: Fetch API
[User impact if declined]: UAF in the Fetch Body consumption.
[Is this code covered by automated tests?]: no, it's racy.
[Has the fix been verified in Nightly?]: we don't have any additional intermittent failure.
[Needs manual test from QE? If yes, steps to reproduce]: none
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not too much.
[Why is the change risky/not risky?]: I just introduced an observer to know when the window goes away. When this happen, Fetch has to stop the consuming of the body.
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8882397 - Flags: approval-mozilla-beta?
Comment on attachment 8882409 [details] [diff] [review]

See previous comment.
Attachment #8882409 - Flags: approval-mozilla-esr52?
I submitted a new patch in bug 1375659 for this crash.
Flags: needinfo?(amarchesini)
Still intending to get this uplifted before b7 gtb?
Flags: needinfo?(jcristau)
Though given bug 1375659, maybe we should just backout bug 1371889 for now. There's some confirmation in the other bug that this is indeed causing real-world crashes too.
Yeah, let's backout this set of changes from beta for now.  Thanks.
Flags: needinfo?(jcristau)
We still get crashes on integration branches. Andrea, should a new bug being filed on those or is there already an existing one?
Flags: needinfo?(amarchesini)
Bug 1375659 fixes this crash. that treeherder push doesn't have that patch yet.
Flags: needinfo?(amarchesini)
OK, so as I understand it, you don't want to uplift this anymore to beta, and I'll look at the patch in bug 1375659 instead. Is that right?
Flags: needinfo?(amarchesini)
We need all 3 marked as blocking bug 1371889.
Comment on attachment 8882397 [details] [diff] [review]

On a second look, it seems like we want both this patch and the one in bug 1375649 on beta. The thread here and in the other bug are a bit confusing.
Attachment #8882397 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8882409 [details] [diff] [review]

Let's also land this on esr52 since it supports the work in bug 1371889.
Attachment #8882409 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
This won't be landing on 55.
Flags: needinfo?(amarchesini)
There was a new occurrence of this issue on autoland today:
Shall this bug get reopened?
Flags: needinfo?(amarchesini)
Bug 1381748 is related to this new crash.
Flags: needinfo?(amarchesini)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.