Closed Bug 1337807 Opened 3 years ago Closed 3 years ago

Some Windows build timeouts could be avoided with SetErrorMode


(Release Engineering :: Applications: MozharnessCore, defect)

Not set


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

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


(Reporter: gbrown, Assigned: gbrown)




(1 file, 1 obsolete file)

Follow-up on 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:
> 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:

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, 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:
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/
@@ +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:


maybe put it after the pre_config_lock meth here:

iow - you could do something like:

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

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
Suppress Windows modal dialogs during builds; r=jlund
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
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.