885472 - Add a Windows event that can be set to trigger Breakpad

Status: RESOLVED WONTFIX

(Toolkit :: Crash Reporting, defect)

Description

[(not currently active) Ted Mielczarek]

This is a WIP. It seems to work fine but probably needs some cleanup. Also: do we want to have this on always, or enable it via an environment variable only in automation?

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: Add a Windows event that can be set to trigger Breakpad

Comment 2

[(not currently active) Ted Mielczarek]

Green on try:
https://tbpl.mozilla.org/?tree=Try&rev=242dbb234a1e

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: use Breakpad event to kill processes on Windows

Too lazy to file a new bug, so this patch replaces the use of crashinject in automation.py with this new event.

Comment 4

[(not currently active) Ted Mielczarek]

Attachment: remove crashinject

Bonus patch: rm the crashinject code and all references to it since we don't need it after these other patches.

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: Add a Windows event that can be set to trigger Breakpad

Fixed the patch so it does cleanup on the handles it creates. Tagging ehsan for review since this is mostly Win32 API fiddling.

The head_crashreporter.js diff is a little weird, I just factored out a bit of the do_crash function into a separate function, but that seems to have really confused diff (it shows the contents of the new function as unchanged, and the rest of do_crash as changed).

This all seems to work great in my local testing, and I have a try push to see if I can get useful info out of it there.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 765915 [details] [diff] [review]
use Breakpad event to kill processes on Windows

The bulk of this patch is calling Win32 APIs via ctypes, so r?ehsan here as well.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 765916 [details] [diff] [review]
remove crashinject

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

I'm not sure why you need my review on this. This is mostly a rubber stamp review.

Comment 8

[(not currently active) Ted Mielczarek]

Just generic "build peer review on touching stuff under build".

Comment 9

[(not currently active) Ted Mielczarek]

Apparently something about these patches was not sufficient, because they failed to do anything useful on WinXP xpcshell hangs. I'm going to try to figure this out in my WinXP VM.

Comment 10

[(not currently active) Ted Mielczarek]

These work fine locally in my WinXP VM, including with an artificially-induced hang in an xpcshell test. This leads me to believe that the root of those xpcshell hangs is something unusual.

Comment 11

[Benoit Girard (:BenWa)]

(In reply to Ted Mielczarek [:ted.mielczarek] (away June 25th-30th) from comment #10)
> These work fine locally in my WinXP VM, including with an
> artificially-induced hang in an xpcshell test. This leads me to believe that
> the root of those xpcshell hangs is something unusual.

Have you tried your patch + artificially-induced hang hang on try? This will rule out that it works only on your VM or just doesn't work in buildbot for some reason.

Comment 12

[(not currently active) Ted Mielczarek]

No, but that's a good idea, so I just pushed that to try:
https://tbpl.mozilla.org/?tree=Try&rev=ace0bb176685

I won't be around to check the results until next week, so if someone cares they can look at that. I added a hang to testing/xpcshell/example/unit/test_sample.js.

Comment 13

[(not currently active) Ted Mielczarek]

That works fine even on WinXP, so this technique works fine in the general case:
https://tbpl.mozilla.org/php/getParsedLog.php?id=24531267&tree=Try

It must be that the hangs we're hitting are during shutdown after we've disabled the crash reporter.

Comment 14

[(not currently active) Ted Mielczarek]

Given that this isn't helping with those WinXP hangs, I'm not sure I want to use this approach at all.

Comment 15

[Benjamin Smedberg]

Can we just use a windbg script to collect a minidump of that process (externally)?

Comment 16

[(not currently active) Ted Mielczarek]

I just wrote some Python to call MiniDumpWriteDump on it, it seems to work fine:
https://hg.mozilla.org/try/rev/978dc9973bd5

I have that in a Try push currently, we'll see how it goes.

Comment 17

[(not currently active) Ted Mielczarek]

Turns out I don't want to do this. I'm going to take the patch on bug 890026 instead.