Closed Bug 1170665 Opened 9 years ago Closed 9 years ago

Prevent the windows error popup in the shell

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox-esr38 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Blocks: 1170124, 1155618
Attached patch no_windows_error_dialog-v0.diff (obsolete) — Splinter Review
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)
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.
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.
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 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 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
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)
I'll uplift this to esr38+ a=NPOTB tomorrow.
Flags: needinfo?(terrence) → needinfo?(ryanvm)
Attachment #8614173 - Attachment is obsolete: true
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.