587747 - Implement writing of minidumps from hang detection on OS X

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[(not currently active) Ted Mielczarek]

In bug 559228 I implemented exception handling for child processes, but not writing of minidumps from the hang detector. We're going to want that too, but it's a little bit more work and I wanted to get the exception handling into 4.0b4.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: almost there

This is close, I just need to fix a couple of things. This works with a test page calling .hang() on the test plugin. Two things:
1) The browser process doesn't wind up with exception info, so it gets no signature, need to fix that
2) Running test_hangning.html in Mochitest hangs.

Comment 2

[(not currently active) Ted Mielczarek]

(In reply to comment #1)
> 2) Running test_hangning.html in Mochitest hangs.

This turned out to be an issue I had previously filed with Breakpad:
http://code.google.com/p/google-breakpad/issues/detail?id=334

so I wrote a patch:
http://breakpad.appspot.com/165001

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: Implement plugin hang dump reporting on OS X

Ok, this works and fixes the issues I mentioned previously. This contains two Breakpad patches that are up for review upstream:
http://breakpad.appspot.com/165001/show
http://breakpad.appspot.com/172001/show

The process_util_mac changes are copied from Chromium. In upstream, the LaunchChild implementation was merged with the POSIX one, we could just take that change wholesale to get slightly more in sync (and reduce our codesize by a little bit), but I figure that could come as a followup, since it'd mean a bit more fiddling to match upstream changes.

Most of the other changes are because on OS X, you need a Mach port to do anything useful to another process (like writing a minidump), and the only way to get one of those is to have it messaged to you via Mach IPC from the other process. To get the task port, we use fork_and_get_task from Chromium, which does the messaging right after forking. To get the thread port, we send the thread index in the IPC message when the plugin inits, and look up the port by index on the parent side. (You can get the list of thread ports given the task port. Mach ports are unique per-process, so you can't send the actual port name across non-Mach IPC.)

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 469045 [details] [diff] [review]
Implement plugin hang dump reporting on OS X

This looks mostly fine to me, though I didn't do a super-close review.

I'm not overjoyed at adding a third kind of Process*/Thread* thing, but I don't have better ideas on hand.  That plus content processes on the horizon probably means the code used to grab subprocess minidumps will want some refactoring, but that's not your fault and doesn't affect beta5.

Comment 5

[(not currently active) Ted Mielczarek]

FWIW, I started down the road of "typedef task_t ProcessHandle", but it wound up being a very invasive change. It certainly makes more sense IMO, but right now we share POSIX code between Linux and OS X all over the place, and lots of it assumes that ProcessHandle == pid_t. We could probably do a two-part refactoring, and audit all users of ProcessHandle to switch ones that want PIDs to use ProcessID instead, and then take the ones that really do need a handle and split the POSIX impl to have a separate OS X path. I wish task_for_pid() would at least work for your own child processes, but it's pretty locked down :-/

Comment 6

[(not currently active) Ted Mielczarek]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/846c67bccbfd

Comment 7

[Josh Aas]

We need to be using 'posix_spawnp' on Mac OS X in order to set the preferred architecture attribute for children, which is necessary for cross-architecture plugin support. This patch moved us in the wrong direction there. See "posix_spawnattr_setbinpref_np".

Comment 8

[(not currently active) Ted Mielczarek]

Well, if you've got a solution that can do what you want and also allow us to get the child's task_t, feel free to implement it.