1694275 - mozprocess waits for output despite exception

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Erickson [:aerickson]]

Attachment: mozprocess_not_returning_immediately_on_exception-live_backing.log

I found a failed browsertime linux job that waited an hour to fail, despite browsertime failing to start and throwing an exception almost immediately. mozprocess ignored the exception and continued to wait for output for an hour.

It seems like mozprocess should exit immediately on an exception. Or should browsertime be using a different functions or args with mozprocess?

Log attached (task https://firefox-ci-tc.services.mozilla.com/tasks/fME8UIgGSROwv6ADaXAaNg/runs/1).

Comment 1

[Geoff Brown [:gbrown]]

afaik, mozprocess makes no promises about its behavior when, as happens here, a line handler (stdoutProcessLine, stdout_callback) throws an exception. It looks to me like the exception is not handled, and the ProcessReader thread ends; other mozprocess threads continue. My initial reaction is that raptor's line handler should not throw an exception, but rather signal mozprocess to end, perhaps with kill()?

Comment 2

[Geoff Brown [:gbrown]]

...and mozprocess should catch and report the exception, but not end...maybe?

Comment 3

[Geoff Brown [:gbrown]]

I can follow-up on those ideas, if no one else is interested.

Comment 4

[Geoff Brown [:gbrown]]

Attachment: Bug 1694275 - Add comment to mozprocess warning about shortcomings; r=

The python subprocess module keeps getting better, while mozprocess is poorly
maintained and subject to various problems (like this bug!), so this patch
adds a comment to mozprocess warning about shortcomings.

Comment 5

[Andrew Erickson [:aerickson]]

(In reply to Geoff Brown [:gbrown] from comment #1)

afaik, mozprocess makes no promises about its behavior when, as happens here, a line handler (stdoutProcessLine, stdout_callback) throws an exception. It looks to me like the exception is not handled, and the ProcessReader thread ends; other mozprocess threads continue. My initial reaction is that raptor's line handler should not throw an exception, but rather signal mozprocess to end, perhaps with kill()?

Agreed. I'll file a raptor bug. Thanks. :)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fe4781d1bdd
Add comment to mozprocess warning about shortcomings; r=jmaher

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/7fe4781d1bdd

Comment 8

[Geoff Brown [:gbrown]]

Attachment: Bug 1694275 - Handle exceptions in mozprocess callbacks; r=

Catch and report any exceptions raised in mozprocess callbacks. This allows the
ProcessReader thread to continue and mozprocess to function normally following
an exception raised in client code.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c7a7a88444f
Handle exceptions in mozprocess callbacks; r=jmaher

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8c7a7a88444f