Closed Bug 1282671 Opened 6 years ago Closed 5 years ago

"Assertion failure: !win->IsDying()" with long-script interrupt and window.close()


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




Tracking Status
firefox50 --- affected
firefox52 --- fixed


(Reporter: jruderman, Assigned: bhsu)


(Keywords: assertion, testcase)


(4 files)

1. Set the prefs mentioned in the testcase
2. Load the testcase
3. Click two buttons

Assertion failure: !win->IsDying(), at /Users/jruderman/trees/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:1456

script terminated by timeout
I can't reproduce when running Firefox under lldb. Does running under lldb disable the script-timeout feature??
Attached file stack
This callback runs arbitrary code, so asserting that the window is not dying seems bogus.  Bobby?
Flags: needinfo?(bobbyholley)
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)
Priority: -- → P2
Hi Ben, would you please help out here?
Flags: needinfo?(bhsu)
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)
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 for more detailed information :)
Assignee: nobody → bhsu
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+
Comment on attachment 8800501 [details] [diff] [review]
Loose the assertion checking win->IsDying()

Per comment 5, I set this patch to r+
Pushed by
Loose the assertion checking win->IsDying(). r=bholly
Keywords: checkin-needed
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.