Closed Bug 1337807 Opened 3 years ago Closed 3 years ago

Some Windows build timeouts could be avoided with SetErrorMode

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(firefox52 fixed, firefox-esr52 fixed, firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

Details

Attachments

(1 file, 1 obsolete file)

Follow-up on https://bugzilla.mozilla.org/show_bug.cgi?id=1318765#c3 and c4:

> One thing we've seen in the past that has caused problems is bad patches
> causing failures to load DLLs, which by default creates a dialog saying
> "Entry Point Not Found", or other crap like that. That will cause the build
> job to hang until it times out. In Firefox itself we call `SetErrorMode` to
> suppress these, but it's possible for it to happen at startup before we get
> to that code, or for us to hit the problem in other tools that don't do
> that. However! I found out today that the error mode is inherited by child
> processes by default, so if we simply did something like this in the
> mozharness script:
> http://stackoverflow.com/a/985166/69326
> 
> We should be able to suppress those as a cause of hangs, at least.

> Specifically (I just tested locally), doing this should suppress those 
> dialogs as well as dialogs from trying to access invalid drive letters:
> ```
> import ctypes
> ctypes.windll.kernel32.SetErrorMode(0x8001)
> ```

I'd like to find a place in mozharness for this, so that the change affects builds in automation, but not local ones.
Somewhere in FxDesktopBuild or BuildScript seems sensible:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/fx_desktop_build.py
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py

I don't know that this needs to be in a separate build step, since it's not running any external commands, but I also don't do much mozharness work so my intuition may not be correct. :)

We shouldn't *need* this for anything but builds--for tests we already have code to capture screenshots on failure, and if an error dialog pops up it's usually visible in the screenshot. Ideally there'd be a way to get the error message from Windows and print it in the log without having a dialog show up that causes everything to hang and timeout, but I don't think that's possible.
I opted for fx_desktop_build.py, before any build steps just in case some early operation runs into trouble. That's seems like a reasonable place to me, but I'm open to other suggestions.

Appears to do no harm: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac2859ce0709a059d3b496a61ceb40a0234cc565
Attachment #8835000 - Flags: review?(jlund)
Comment on attachment 8835000 [details] [diff] [review]
call SetErrorMode on Windows builds

Review of attachment 8835000 [details] [diff] [review]:
-----------------------------------------------------------------

looks good bar the location of it. Feel free to push back if this doesn't work or you don't think it's a good idea :)

::: testing/mozharness/scripts/fx_desktop_build.py
@@ +103,5 @@
>          }
> +        if self._is_windows():
> +            # Suppress Windows modal dialogs to avoid hangs
> +            import ctypes
> +            ctypes.windll.kernel32.SetErrorMode(0x8001)

I tried to keep init free from too much script logic. This feels like a good candidate for steps that are required prior to any actions/steps run in mozharness. Could you try using preScriptRun:

def: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#1665
example: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/test/test_base_script.py#721

maybe put it after the pre_config_lock meth here: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py?q=path%3Abuildbase&redirect_type=single#644


iow - you could do something like:

```
@script.PreScriptRun
    def suppress_windows_modal_dialogs(self, *args, **kwargs):
         if self._is_windows(): # etc
```
Attachment #8835000 - Flags: review?(jlund) → review-
That works for me:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e4d9d176a135c8db00c32b08141ac411e45553f

19:20:23     INFO - Running pre-run listener: suppress_windows_modal_dialogs
Attachment #8835000 - Attachment is obsolete: true
Attachment #8837286 - Flags: review?(jlund)
Comment on attachment 8837286 [details] [diff] [review]
call SetErrorMode on Windows builds

thanks! lgtm
Attachment #8837286 - Flags: review?(jlund) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/56318bc8af9b
Suppress Windows modal dialogs during builds; r=jlund
https://hg.mozilla.org/mozilla-central/rev/56318bc8af9b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-aurora/rev/df0a213dad6c
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.