Windows 32-bit Crash in [@ _chkstk | nsBoxFrame::BuildDisplayList]
Categories
(Toolkit :: General, defect)
Tracking
()
People
(Reporter: aryx, Unassigned)
Details
(Keywords: crash)
Crash Data
Not a new signature, all crashes with 32-bit builds on Windows.
Crash report: https://crash-stats.mozilla.org/report/index/7c985201-1950-42fd-80d2-50e920220221
Reason: EXCEPTION_STACK_OVERFLOW
Top 10 frames of crashing thread:
0 xul.dll _chkstk /builds/worker/workspace/obj-build/toolkit/library/build/d:/agent/_work/3/s/src/vctools/crt/vcstartup/src/misc/i386/chkstk.asm:99
1 xul.dll nsBoxFrame::BuildDisplayList layout/xul/nsBoxFrame.cpp:958
2 xul.dll nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4375
3 xul.dll nsBoxFrame::BuildDisplayListForChildren layout/xul/nsBoxFrame.cpp:998
4 xul.dll nsBoxFrame::BuildDisplayList layout/xul/nsBoxFrame.cpp:958
5 xul.dll nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4375
6 xul.dll nsBoxFrame::BuildDisplayListForChildren layout/xul/nsBoxFrame.cpp:998
7 xul.dll nsBoxFrame::BuildDisplayList layout/xul/nsBoxFrame.cpp:958
8 xul.dll nsIFrame::BuildDisplayListForChild layout/generic/nsIFrame.cpp:4393
9 xul.dll nsFlexContainerFrame::BuildDisplayList layout/generic/nsFlexContainerFrame.cpp:2944
|
||
Comment 1•4 years ago
|
||
So all of these are in deep nested event loop calls:
XPCOMSpinEventLoopStack: prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync|prompts/Prompter.jsm:openPromptSync
:pbz, it seems we can open nested prompts that never close, in a way that blows up our stack enough to affect win32 users. Are we expected to open nested modal prompts? If not, perhaps we can bail out when already showing other modal prompts or something?
|
||
Comment 2•4 years ago
|
||
Interesting find!
I'm trying to understand what nested modal prompts are. Does nested event loops mean that it has to be nested prompts, e.g. an open prompt spinning a nested event loop and then opening another from there? Or could this be caused simply by making a lot of prompt calls, for example triggered by web content?
Off-hand I'm not aware of any outstanding bugs where sites can spam prompts. Usually we pause JavaScript execution, which prevents any further calls while a prompt is open.
Gijs, you have worked on the embedded window prompts previously, do you remember if we can into issues similar to this?
Can we somehow find out where these calls are coming from?
|
||
Comment 3•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
:pbz, it seems we can open nested prompts that never close, in a way that blows up our stack enough to affect win32 users. Are we expected to open nested modal prompts? If not, perhaps we can bail out when already showing other modal prompts or something?
I think if you have 2 windows, 1 window can just continuously open alert()s in another window, from e.g. a setTimeout(..., 0), and this would lead to this kind of behaviour. Per spec, we need to have a return value from the alert (or prompt or confirm) synchronously. Opening the prompt is an async operation. AIUI the only way to do this is to spin a new event loop; there is no way to "piggyback" on the previous / pre-existing event loop.
It may be possible in a single window if you trigger the alerts from event handlers or other code that doesn't end up frozen out by the fact that the global window has entered modal state (which IIRC will pause event dispatch to the DOM of that window).
I think we have existing bugs on file about this, and the not-quite-per-spec behaviour of alert and friends. Olli would know more.
Now that we're fully fission, we could consider actually freezing the entire content process instead of spinning an event loop, but I bet that would still lead to unexpected consequences... and it's not a trivial engineering investment.
The other alternative would be throwing an exception if an alert is already up? But most websites won't be set up for that; alert isn't meant to throw, I think... We could also return and, for prompt and confirm, lie about the return value, but again not sure what the user impact would be.
Finally of course it's possible we have some kind of bug where we don't unwind the event loop once the dialog goes away? I think I've seen issues like this where, if you first open dialog A and then dialog B, if dialog A closes we still can't unwind the loop for A because we're in the loop for B...
|
||
Comment 4•4 years ago
|
||
I don't think we'd want to freeze a child process. We want to be able to still destroy it in some reasonable way.
For alert()s we could just return early if there is another open already I think.
Doesn't Chrome do something similar to that in some cases.
I think nika was testing Chrome's behavior at some point.
|
|
||
Comment 5•4 years ago
|
||
Oh, hm, except these are parent process crashes.
In that case, you'd need to look at places that use synchronous APIs to open these; there are some (e.g. http auth, and things like bug 1746089)
|
||
Comment 6•4 years ago
|
||
Yeah(In reply to Olli Pettay [:smaug] from comment #4)
I don't think we'd want to freeze a child process. We want to be able to still destroy it in some reasonable way.
For alert()s we could just return early if there is another open already I think.
Doesn't Chrome do something similar to that in some cases.
I think nika was testing Chrome's behavior at some point.
Yeah, chrome is fairly aggressive in it's modifications to alert dialog semantics IIRC. I found this old document though I don't know how up-to-date it is anymore: https://docs.google.com/document/d/1wtV5rmLhbf1OZkbg7crtCt6h1mMtig_ctTQt3BLLEIU/edit
The rough idea is that alert in non-foreground tabs actually doesn't block & instantly returns, as does confirm/prompt in anything but the foreground tab, returning a cancelled response. This avoids the need for nested event loops in almost all situations (though IIRC chrome actually never uses nested event loops for alerts, and instead blocks the entire content process, as they clear the wait as soon as you try to interact with something).
If we could do a change like this it might let us dramatically simplify our alert setup, avoiding a lot of the issues which nested event loops come with. I don't know if we'd be able to get away with blocking instead of nested event loops though - that might be a bigger project (though more tractable due to Fission).
|
|
||
Comment 7•4 years ago
|
||
Do we know how/why the 32-bit part of this impacts us for nested event loops?
|
|
||
Comment 8•4 years ago
|
||
(put differently, if this crashes, should we just fail out of JS nested event loops with exceptions once we hit a certain depth? That seems preferable over crashing, certainly!)
|
|
||
Comment 9•4 years ago
|
||
32-bit builds have less virtual memory and thus probably smaller stacks, not sure if that's what you were asking?
In terms of "can we just throw"... Maybe? The JS engine does have some stack limit checks, though not sure if they apply to this.
|
||
Comment 10•3 years ago
|
||
Closing because no crashes reported for 12 weeks.
Description
•