Open Bug 1789231 Opened 2 years ago Updated 1 year ago

[meta] Investigate potentially long running child process main thread runnables

Categories

(Core :: DOM: Content Processes, task)

task

Tracking

()

People

(Reporter: jstutte, Unassigned)

References

(Depends on 5 open bugs, Blocks 1 open bug)

Details

(Keywords: meta)

One major source of shutdown hangs are runnables that may block the main thread for a long time, preventing us from ever even processing the shutdown message we received by the parent process, as indicated by the IPCShutdownState containing NotifiedImpendingShutdown but no other progress.

The shutdown message is just queued in the main thread event queue as any other message, and there are basically two different scenarios here after we enqueued the shutdown message:

  • While we are processing preceding events, we encounter a long running runnable that will block the main thread.
  • We are already processing such a runnable when we enqueue the shutdown message

The distinction is important, as in the first case we can just easily check at the beginning of a runnable, if we are shutting down, in the latter case we need to have means to interrupt the execution. Unfortunately we cannot really distinguish the two cases from our received crashes, but from the investigation in bug 1755376 it is expected that the latter case is much more frequent. See bug 1777198 for an example where we want to interrupt such a runnable in case of shutdown.

There is also a special case where we do not block the main thread execution but wait inside a SpinEventLoopUntil until something else unblocks, effectively preventing the main thread from a regular shutdown, too. See bug 1740889 for an example.

It is worth repeating here, that the stack of those child process will not show signs of shutdown yet, but we can check if we are asked to shutdown with mozilla::ipc::ProcessChild::ExpectingShutdown() which will be true only in child processes as soon as they were NotifiedImpendingShutdown, called through the IPC I/O thread. This is actually a special message intercepted by MessageChannel::MaybeInterceptSpecialIOMessage, so it bypasses all queues and the main thread. (Please note that parent process relevant checks should be based on AppShutdown::IsInOrBeyond instead).

Please note also that blocking shutdown is just one possible side effect of such heavy-weight runnables. Moving them off-mainthread and/or limiting there maximum lifetime where possible would be probably a good thing for the overall performance, not just for shutdown.

Depends on: 1788879
Depends on: 1777198
Depends on: 1740889

(In reply to Jens Stutte [:jstutte] from comment #0)

  • We are already processing such a runnable when we enqueue the shutdown message
    ...
    we can check if we are asked to shutdown with mozilla::ipc::ProcessChild::ExpectingShutdown() which will be true only in child processes as soon as they were NotifiedImpendingShutdown

For my understanding, if we're already processing a runnable, how does NotifiedImpendingShutdown get called? Is it called on a background thread? I just want to be sure I understand what needs to be done here; i.e. whether checking ExpectingShutdown in various places is enough.

(In reply to James Teh [:Jamie] from comment #1)

(In reply to Jens Stutte [:jstutte] from comment #0)

  • We are already processing such a runnable when we enqueue the shutdown message
    ...
    we can check if we are asked to shutdown with mozilla::ipc::ProcessChild::ExpectingShutdown() which will be true only in child processes as soon as they were NotifiedImpendingShutdown

For my understanding, if we're already processing a runnable, how does NotifiedImpendingShutdown get called? Is it called on a background thread? I just want to be sure I understand what needs to be done here; i.e. whether checking ExpectingShutdown in various places is enough.

Yes, it is called through the IPC I/O thread. It is actually a special message intercepted by MessageChannel::MaybeInterceptSpecialIOMessage, so it bypasses all queues and the main thread.

Edit: Updated description, thanks for the feedback.

Depends on: 1789422
Depends on: 1789425

We could make Process Shutdown messages a high-priority event, which handles one of the cases in comment 0.

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #3)

We could make Process Shutdown messages a high-priority event, which handles one of the cases in comment 0.

We did this investigation in bug 1755376, the problem here are (at least in some cases) already running runnables such that we never get back to spin the event loop.

Edit: Updated description, thanks for the feedback.

Depends on: 1789807
No longer depends on: 1789422
Depends on: 1790360
Depends on: 1809115
Depends on: 1813602
You need to log in before you can comment on or make changes to this bug.