1620619 - Improve shutdown annotations for content processes

Status: RESOLVED WORKSFORME

(Core :: DOM: Content Processes, task, P2)

Description

[Andrew McCreight [:mccr8]]

In bug 1616018 comment 0, gsvelto said "Content process shutdown can be detected by listening to the "content-child-will-shutdown" notification", but it looks like content-child-will-shutdown is not in nsTerminator's sShutdownSteps, so I think the shutdown progress crash report annotation won't ever tell us when the content process has begun to shut down. Maybe we could annotate that?

In the current state, it makes it hard to tell things like bug 1619455 are actually doing anything, because I can't tell if these GCs were already in progress when we got the will-shutdown or if my patch somehow is not really preventing them.

Comment 1

[Andrew McCreight [:mccr8]]

Maybe IPCShutdownState is actually sufficient for seeing what is going on here.

Comment 2

[Gabriele Svelto [:gsvelto]]

IPCShutdownState is a good proxy. There's 4 states to i ATM:

• The annotation is not present: the content process hasn't received the shutdown IPC message yet (or it has received it but the event loop didn't pull it out of the queue)
• The annotation is set to RecvShutdown: the content process has received the shutdown IPC message and started responding to it
• The annotation is set to SendFinishShutdown (sending): the content process has finished shutting down and is about to send an IPC message to inform the parent
• The annotation is set to SendFinishShutdown (sent): the message to inform the parent was sent. This can also be SendFinishShutdown (failed) in case of an IPC failure

There's a wrinkle in this though: since the timeout the parent process uses to kill the content one is decoupled from the IPC messages it's possible for the minidump of a ShutDownKill crash to show that the content process has already finished shutdown. This happens when the IPC message sent by the content process shows up in the event queue later than the kill timer.

Anyway it would be nice if we had a more incremental mechanism for the parent process to check the shutdown state of a content process and decide if it's stuck or not.

Comment 3

[Chris Peterson [:cpeterson]]

P2
kmag says this change would be nice to have and should be easy to make.

Comment 4

[Jens Stutte [:jstutte]]

I think the recent improvements from https://phabricator.services.mozilla.com/D155814 should give us quite some information here (aggregating on IPC shutdown state in crashstats). :mccr8, do you think we can close this bug?

Comment 5

[Andrew McCreight [:mccr8]]

Thanks to your work on shutdown hangs, I haven't been paying attention to them in a long time, so if you are fine with the state of things, I'm fine with it.