Closed Bug 1374922 Opened 6 years ago Closed 6 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed

People

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

References

Details

(Keywords: crash, intermittent-failure, regression)

Crash Data

Attachments

(4 files, 1 obsolete file)

part 1 - main-thread
6.09 KB, patch
bkelly
: review+
Details | Diff | Splinter Review
part 2 - workers
5.84 KB, patch
bkelly
: review+
Details | Diff | Splinter Review
m-b
5.32 KB, patch
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
m-esr52
9.08 KB, patch
lizzard
: approval-mozilla-esr52+
Details | Diff | Splinter Review 
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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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  libxul.so!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?
status-firefox56: --- → affected
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 - workers — — Splinter Review 
We should also listen for xpcom-shutdown in workers and main-thread.
Attachment #8879869 - Flags: review?(bkelly)
Status: NEW → ASSIGNED
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 amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e71b420e295d
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.
https://hg.mozilla.org/mozilla-central/rev/e71b420e295d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox56: affected → fixed
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?

https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=mn&bugfiler&fromchange=540e811a9a1118ca87569d08a47780c3c07ae402&selectedJob=110381398
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?

https://treeherder.mozilla.org/logviewer.html#?job_id=110711389&repo=mozilla-beta
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-b
        — 
          — Splinter Review
      

    


  
Patch rebased

        Attachment #8882351 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        m-esr52
        — 
          — Splinter 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).
Blocks: CVE-2017-7793
Flags: needinfo?(amarchesini)

    
    

    
  
Comment on attachment 8882397 [details] [diff] [review]
m-b

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]
m-esr52

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?

https://treeherder.mozilla.org/logviewer.html#?repo=autoland&job_id=112571113&lineNumber=42929
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]
m-b

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]
m-esr52

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.

          status-firefox55:
          affected → unaffected
Flags: needinfo?(amarchesini)

    
    

    
  
There was a new occurrence of this issue on autoland today: https://treeherder.mozilla.org/logviewer.html#?job_id=115866638&repo=autoland
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.