"Assertion failure: !win->IsDying()" with long-script interrupt and window.close()
RESOLVED
FIXED
in Firefox 52
Status
()
People
(Reporter: jruderman, Assigned: bhsu)
Tracking
(Blocks: 2 bugs, {assertion, testcase})
Firefox Tracking Flags
(firefox50 affected, firefox52 fixed)
Details
Attachments
(4 attachments)
Created attachment 8765741 [details]
testcase (manual version - click two buttons)
1. Set the prefs mentioned in the testcase
2. Load the testcase
3. Click two buttons
Result:
Assertion failure: !win->IsDying(), at /Users/jruderman/trees/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:1456
Expected:
script terminated by timeout
(Reporter) | ||
Comment 1•3 years ago
|
||
Created attachment 8765742 [details]
testcase (automated version)
(Reporter) | ||
Comment 2•3 years ago
|
||
I can't reproduce when running Firefox under lldb. Does running under lldb disable the script-timeout feature??
(Reporter) | ||
Comment 3•3 years ago
|
||
Created attachment 8765743 [details]
stack
This callback runs arbitrary code, so asserting that the window is not dying seems bogus. Bobby?
Flags: needinfo?(bobbyholley)
Comment 5•3 years ago
|
||
Yeah, we should probably just check and bail out like we do for the non-window case, though probably in this case we should return false and kill the script. r=me on a patch that replaces the assert with: if (win->IsDying()) { // The window is being torn down. When that happens we try to prevent the dispatch of new runnables, // so it also makes sense to kill any long-running script. The user is primarily interested in this // page going away. return false; }
Flags: needinfo?(bobbyholley)
Updated•3 years ago
|
Priority: -- → P2
(Assignee) | ||
Comment 7•3 years ago
|
||
Sure, but I am new to this field, it would take me a while, and thus I will working on this without taking it :)
Flags: needinfo?(bhsu)
(Assignee) | ||
Comment 8•2 years ago
|
||
Hi Jesse, and sorry for such a late response After diving into the code a little bit, I found that the case is still reproducible by executing firefox in non-e10s mode, which you can do that by the following command. ``` ./mach run --disable-e10s ``` With e10s enabled, it seems that Gecko doesn't immediately close the nsGlobalWindow after invoking `window.close()`, and thus the nsGlobalWindow is not marked as dying. As a result, we don't get hit by the assertion. Please refer to http://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#8607-8609 for more detailed information :)
(Assignee) | ||
Updated•2 years ago
|
Assignee: nobody → bhsu
(Assignee) | ||
Comment 9•2 years ago
|
||
Created attachment 8800501 [details] [diff] [review] Loose the assertion checking win->IsDying()
(Assignee) | ||
Comment 10•2 years ago
|
||
Comment on attachment 8800501 [details] [diff] [review] Loose the assertion checking win->IsDying() Per comment 5, I set the flag to r+
Attachment #8800501 -
Flags: review+
(Assignee) | ||
Comment 11•2 years ago
|
||
Comment on attachment 8800501 [details] [diff] [review] Loose the assertion checking win->IsDying() Per comment 5, I set this patch to r+
(Assignee) | ||
Comment 12•2 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aec25af40696
Keywords: checkin-needed
Comment 13•2 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba66a8bb5b3d Loose the assertion checking win->IsDying(). r=bholly
Keywords: checkin-needed
Comment 14•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ba66a8bb5b3d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•