1274444 - Windows builds hang when sending completion notification

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Bill Gianopoulos [:WG9s]]

Attachment: my workaround patch

My windows builds often hang after reporting the number of compiler warnings and before the message about "It took a while ..."
ON a whim I tried disabling attempts to notify on windows builds and it cleared the issue.

I will attach the patch I am using.

Comment 1

[Bill Gianopoulos [:WG9s]]

But, of course, that does not mean this should not be a bug filed against Gnome 3 or it's default adwaita theme.

Comment 2

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #1)
> But, of course, that does not mean this should not be a bug filed against
> Gnome 3 or it's default adwaita theme.

Ooops comment added to wrong bug sorry.

Comment 3

[Bill Gianopoulos [:WG9s]]

There should be some way to make this not try to notify, or at least not hang if this is a non-interactive process.

Comment 4

[Gregory Szorc [:gps]]

Please this code into e.g. `test.py` and run with `python test.py` and report what happens. Also, is this inside stock MozillaBuild? Or are you using a fancy terminal?

from ctypes import Structure, windll, POINTER, sizeof
from ctypes.wintypes import DWORD, HANDLE, WINFUNCTYPE, BOOL, UINT
class FLASHWINDOW(Structure):
    _fields_ = [("cbSize", UINT),
                ("hwnd", HANDLE),
                ("dwFlags", DWORD),
                ("uCount", UINT),
                ("dwTimeout", DWORD)]
FlashWindowExProto = WINFUNCTYPE(BOOL, POINTER(FLASHWINDOW))
FlashWindowEx = FlashWindowExProto(("FlashWindowEx", windll.user32))
FLASHW_CAPTION = 0x01
FLASHW_TRAY = 0x02
FLASHW_TIMERNOFG = 0x0C
params = FLASHWINDOW(sizeof(FLASHWINDOW),
                    windll.kernel32.GetConsoleWindow(),
                    FLASHW_CAPTION | FLASHW_TRAY | FLASHW_TIMERNOFG, 3, 0)
FlashWindowEx(params)

Comment 5

[Gregory Szorc [:gps]]

There is an environment variable to make it not notify (MOZ_NOSPAM).

But the proper fix is probably to look at sys.stdin.isatty() and not notify unless that's true. However, that will also suppress notifications if you e.g. pipe output or not perform invocation from a terminal.

Comment 6

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #5)
> There is an environment variable to make it not notify (MOZ_NOSPAM).
> 
> But the proper fix is probably to look at sys.stdin.isatty() and not notify
> unless that's true. However, that will also suppress notifications if you
> e.g. pipe output or not perform invocation from a terminal.

I did not say this was a proper fix was really looking for a more proper fix.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #4)
> Please this code into e.g. `test.py` and run with `python test.py` and
> report what happens. Also, is this inside stock MozillaBuild? Or are you
> using a fancy terminal?
> 
> from ctypes import Structure, windll, POINTER, sizeof
> from ctypes.wintypes import DWORD, HANDLE, WINFUNCTYPE, BOOL, UINT
> class FLASHWINDOW(Structure):
>     _fields_ = [("cbSize", UINT),
>                 ("hwnd", HANDLE),
>                 ("dwFlags", DWORD),
>                 ("uCount", UINT),
>                 ("dwTimeout", DWORD)]
> FlashWindowExProto = WINFUNCTYPE(BOOL, POINTER(FLASHWINDOW))
> FlashWindowEx = FlashWindowExProto(("FlashWindowEx", windll.user32))
> FLASHW_CAPTION = 0x01
> FLASHW_TRAY = 0x02
> FLASHW_TIMERNOFG = 0x0C
> params = FLASHWINDOW(sizeof(FLASHWINDOW),
>                     windll.kernel32.GetConsoleWindow(),
>                     FLASHW_CAPTION | FLASHW_TRAY | FLASHW_TIMERNOFG, 3, 0)
> FlashWindowEx(params)

This is just a stock build the only thing I do is to make a small modification to the start....bat files in mozilla_build to allow including a shell script to run as a parameter so I can get the builds to work via the windows scheduler.  perhaps I should file a separate bug to get that added to make it a standard feature.

Comment 8

[Bill Gianopoulos [:WG9s]]

Oh and the failure is not a consistent thing.  i do a 32-bit and 64-bit windows build sometimes they both work most days one or the other does often they both fail i wi=would say it hangs at least 30% of the time.

Comment 9

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #7)
> (In reply to Gregory Szorc [:gps] from comment #4)
> > Please this code into e.g. `test.py` and run with `python test.py` and
> > report what happens. Also, is this inside stock MozillaBuild? Or are you
> > using a fancy terminal?
> > 
> > from ctypes import Structure, windll, POINTER, sizeof
> > from ctypes.wintypes import DWORD, HANDLE, WINFUNCTYPE, BOOL, UINT
> > class FLASHWINDOW(Structure):
> >     _fields_ = [("cbSize", UINT),
> >                 ("hwnd", HANDLE),
> >                 ("dwFlags", DWORD),
> >                 ("uCount", UINT),
> >                 ("dwTimeout", DWORD)]
> > FlashWindowExProto = WINFUNCTYPE(BOOL, POINTER(FLASHWINDOW))
> > FlashWindowEx = FlashWindowExProto(("FlashWindowEx", windll.user32))
> > FLASHW_CAPTION = 0x01
> > FLASHW_TRAY = 0x02
> > FLASHW_TIMERNOFG = 0x0C
> > params = FLASHWINDOW(sizeof(FLASHWINDOW),
> >                     windll.kernel32.GetConsoleWindow(),
> >                     FLASHW_CAPTION | FLASHW_TRAY | FLASHW_TIMERNOFG, 3, 0)
> > FlashWindowEx(params)
> 
> This is just a stock build the only thing I do is to make a small
> modification to the start....bat files in mozilla_build to allow including a
> shell script to run as a parameter so I can get the builds to work via the
> windows scheduler.  perhaps I should file a separate bug to get that added
> to make it a standard feature.

Oh and you have obviously confused my with someone who knows enough about python to understand what you just asked me to try.

Comment 10

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #5)
> There is an environment variable to make it not notify (MOZ_NOSPAM).
> 
> But the proper fix is probably to look at sys.stdin.isatty() and not notify
> unless that's true. However, that will also suppress notifications if you
> e.g. pipe output or not perform invocation from a terminal.

but then where was the notify going to display anyway in those other cases?

I have a better patch coming. that perhaps is what we need.

Comment 11

[Gregory Szorc [:gps]]

Paste the Python code into a new file - say test.py - then run it with `python test.py` from a MozillaBuild shell. I'm curious if that randomly hangs, throws an exception, etc.

Comment 12

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1274444 - Check for null console before attempting to flash it; r?glandium

GetConsoleWindow() can return NULL. Previously, we may have passed a NULL
console reference into FlashWindowEx(). On my machine (when running in a
console), passing NULL doesn't seem to cause an error. But since we have
a report of this function hanging, it is quite possible it can cause
hangs in other scenarios. Since a NULL console won't result in any
notification, let's not call FlashWindowEx() when no console is available.

Review commit: https://reviewboard.mozilla.org/r/54338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54338/

Comment 13

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #12)
> Created attachment 8755022 [details]
> MozReview Request: Bug 1274444 - Check for null console before attempting to
> flash it; r?glandium
> 
> GetConsoleWindow() can return NULL. Previously, we may have passed a NULL
> console reference into FlashWindowEx(). On my machine (when running in a
> console), passing NULL doesn't seem to cause an error. But since we have
> a report of this function hanging, it is quite possible it can cause
> hangs in other scenarios. Since a NULL console won't result in any
> notification, let's not call FlashWindowEx() when no console is available.
> 
> Review commit: https://reviewboard.mozilla.org/r/54338/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/54338/

I have changed my automated builds to include this patch I will let you know the results (since I seem to b the only person seeing this issue).

Comment 14

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #11)
> Paste the Python code into a new file - say test.py - then run it with
> `python test.py` from a MozillaBuild shell. I'm curious if that randomly
> hangs, throws an exception, etc.

except if i am cutting and pasting and running then I am in a n interactive session where this does not hang.

Comment 15

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #13)
> (In reply to Gregory Szorc [:gps] from comment #12)
> > Created attachment 8755022 [details]
> > MozReview Request: Bug 1274444 - Check for null console before attempting to
> > flash it; r?glandium
> > 
> > GetConsoleWindow() can return NULL. Previously, we may have passed a NULL
> > console reference into FlashWindowEx(). On my machine (when running in a
> > console), passing NULL doesn't seem to cause an error. But since we have
> > a report of this function hanging, it is quite possible it can cause
> > hangs in other scenarios. Since a NULL console won't result in any
> > notification, let's not call FlashWindowEx() when no console is available.
> > 
> > Review commit: https://reviewboard.mozilla.org/r/54338/diff/#index_header
> > See other reviews: https://reviewboard.mozilla.org/r/54338/
> 
> I have changed my automated builds to include this patch I will let you know
> the results (since I seem to b the only person seeing this issue).

So far this seems to fix the issue and looks like the correct fix.  I will report back on Monday at around 10AM eastern time.  would mean 8 successful builds in a row by that time.

Comment 16

[Bill Gianopoulos [:WG9s]]

OK.  8 consecutive successful builds.  I would say the patch here corrects the issue.

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 8755022 [details]
MozReview Request: Bug 1274444 - Check for null console before attempting to flash it; r?glandium

https://reviewboard.mozilla.org/r/54338/#review51326

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29ff5e9de6d7

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/29ff5e9de6d7

Comment 20

[Bill Gianopoulos [:WG9s]]

This has gone a couple of weeks without a recurrence of the issue.  Verifying this as fixed.