1060527 - We don't check for crashes if we get an exception in ProcessHandler

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Jonathan Griffin (:jgriffin)]

When a job times out, we normally deliberately crash the process so we can get a stack trace from it.  See https://tbpl.mozilla.org/php/getParsedLog.php?id=46820280&tree=Mozilla-Central&full=1 for an example of this working normally on Windows (search for 'crashinject').

On some jobs, though, it looks our deliberate crash is causing mozprocess to throw an error, and when this happen we don't invoke mozcrash.  See bug 900112 and https://tbpl.mozilla.org/php/getParsedLog.php?id=46961924&tree=Fx-Team (again, search for 'crashinject').

Comment 1

[Jonathan Griffin (:jgriffin)]

Ahal, can you take a look?  Let me know if it's not an easy fix.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: mozprocess_signal_completion

I don't understand this IO Completion Port stuff very well, nor do I have a windows environment handy to test on. Just based on the logs and source code though, I think this patch might fix the issue. I have no idea why this is only happening some of the time though and I also don't know if this will have undesired side effects.

Here's a try run:
https://tbpl.mozilla.org/?tree=Try&rev=a17103a474a5

Comment 3

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8482748 [details] [diff] [review]
mozprocess_signal_completion

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

This looks reasonable to me as well.  I think there's a race condition somewhere with our handling of Windows processes, such that killing a process (as with crashinject) can sometimes trigger this code path.  This won't fix the race condition, but will hopefully render it harmless.

Comment 4

[Andrew Halberstadt [:ahal]]

So sorry, this completely slipped past me. Thank you harth for your bugzilla todo app that found this.

Here's a try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=043cba306d20

Looked good, so landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/70a6139f5517

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/70a6139f5517