Closed
Bug 1170665
Opened 9 years ago
Closed 9 years ago
Prevent the windows error popup in the shell
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file, 1 obsolete file)
1.14 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
Jit-tests appear to be timing out because of the popup that occurs when we crash. This is particularly bad when combined with certain allow-oom tests that expect to die with a segfault.
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 1•9 years ago
|
||
This was sufficient to fix the the OOM test that is failing in bug 1170124. The API here allows us to mask other sorts of error dialogs as well, but I'm not sure which of them is relevant and when.
Attachment #8614173 -
Flags: review?(jdemooij)
Comment 2•9 years ago
|
||
Interesting. It says about SEM_FAILCRITICALERRORS: "Best practice is that all applications call the process-wide SetErrorMode function with a parameter of SEM_FAILCRITICALERRORS at startup. This is to prevent error mode dialogs from hanging the application." So that sounds like a reasonable one to add as well, assuming it isn't already set. SEM_NOOPENFILEERRORBOX doesn't sound unreasonable either - if we can't find a file we'd probably want to handle that ourselves, right? The only one that sounds potentially dangerous is SEM_NOALIGNMENTFAULTEXCEPT, so I'd suggest calling SetErrorMode with SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX | SEM_NOOPENFILEERRORBOX. I'm not speaking from experience though, I didn't even know this API existed before.
Assignee | ||
Comment 3•9 years ago
|
||
Yes, exactly what I was thinking. I eventually uploaded the minimal solution instead because who knows what releng has done to the windows machines running our tests? I'm teetering back now though and thinking we might as well just go all-in here. Worst case is not bad and just as likely it'd save us more time in the future.
Assignee | ||
Comment 4•9 years ago
|
||
Maybe just handle all dialogs right now rather than having to revisit this later. Seems to be working fine locally.
Attachment #8614173 -
Attachment is obsolete: true
Attachment #8614173 -
Flags: review?(jdemooij)
Attachment #8614220 -
Flags: review?(jdemooij)
Comment 5•9 years ago
|
||
Comment on attachment 8614173 [details] [diff] [review] no_windows_error_dialog-v0.diff Review of attachment 8614173 [details] [diff] [review]: ----------------------------------------------------------------- Awesome, thanks for looking into this! Agreed with the last two comments; adding the other flags except NOALIGNMENTFAULTEXCEPT seems fine.
Attachment #8614173 -
Attachment is obsolete: false
Comment 6•9 years ago
|
||
Comment on attachment 8614220 [details] [diff] [review] no_windows_error_dialog-v1.diff Review of attachment 8614220 [details] [diff] [review]: ----------------------------------------------------------------- Oops, we mid-aired.
Attachment #8614220 -
Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/617e217524a2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
This might explain some of the error dialogs I get on Windows when fuzzing, some of which I could disable in the Windows registry but some which still frustrate. Hopefully they will be gone moving forward. Can we please backport this shell-only fix? It will help ease fuzzing pains.
Flags: needinfo?(terrence)
(and also makes less jit-tests timeout, as per comment 0)
Comment 11•9 years ago
|
||
I'll uplift this to esr38+ a=NPOTB tomorrow.
Flags: needinfo?(terrence) → needinfo?(ryanvm)
Updated•9 years ago
|
Attachment #8614173 -
Attachment is obsolete: true
Updated•9 years ago
|
Flags: needinfo?(ryanvm)
Comment 13•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc33e774e56d
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•