1477393 - Replace various watchdog/hang monitor threads with timers

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Kris Maglione [:kmag]]

We currently have 4 watchdog/hang monitor threads per content process in nightly and beta populations:

│     ├────8,192 B (00.04%) ── BHMgr Monitor (tid=8360)
│     ├────8,192 B (00.04%) ── BHMgr Processor (tid=8361)
│     ├────8,192 B (00.04%) ── JS Watchdog (tid=8345)
│     ├────8,192 B (00.04%) ── ProcessHangMon (tid=8441)

Three of these can almost certainly be converted to timers which dispatch to a shared thread pool. BHMgr Processor can probably just use a thread pool directly.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Kris Maglione [:kmag] from comment #0)
> Three of these [4 watchdog/hang monitor threads] can almost certainly be
> converted to timers which dispatch to
> a shared thread pool. BHMgr Processor can probably just use a thread pool
> directly.

What if the thread pool is blocked? It should be rare, but I've seen it happen with the media-playback thread pool, when we had a "dining philosophers" bug and lots of videos!
I think watchdogs that are tasked with terminating processes in case of (probably-)blocking bugs should have at least one thread to themselves.

Comment 2

[Kris Maglione [:kmag]]

(In reply to Gerald Squelart [:gerald] from comment #1)
> What if the thread pool is blocked? It should be rare, but I've seen it
> happen with the media-playback thread pool, when we had a "dining
> philosophers" bug and lots of videos!
> I think watchdogs that are tasked with terminating processes in case of
> (probably-)blocking bugs should have at least one thread to themselves.

I was leaning towards having a priority queue, with watchdog runnables having the highest priority, and possibly some provision for spinning up a new thread for critical events if the rest of the pool is blocked.

Each thread on Windows uses a minimum of 32K, and we already need at least one timer thread, so I'm reluctant to add any more always-running-usually-idle threads to every content process if we can possibly avoid it.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Kris Maglione [:kmag] from comment #0)
> We currently have 4 watchdog/hang monitor threads per content process in
> nightly and beta populations:
> 
> │     ├────8,192 B (00.04%) ── BHMgr Monitor (tid=8360)
> │     ├────8,192 B (00.04%) ── BHMgr Processor (tid=8361)
> │     ├────8,192 B (00.04%) ── JS Watchdog (tid=8345)
> │     ├────8,192 B (00.04%) ── ProcessHangMon (tid=8441)
> 
> Three of these can almost certainly be converted to timers which dispatch to
> a shared thread pool. BHMgr Processor can probably just use a thread pool
> directly.

BHMgr Monitor does some weird timer-ish stuff, and while it should almost certainly be rewritten, I'm a bit dubious as to how worthwhile that excercise is. It's not a great system right now & I spent a lot of time while working on it hunting down weird bugs :-/. (Turns out running a "profiler" in the background is kinda error-prone...)

One option which could be interesting would be for the parent process to handle the profiling of content processes, pausing them & walking their stacks when they aren't clearing out their IPC message queue fast enough.

BHMgr Processor actually used to be in a thread pool (it used to use the StreamTransportService), but it caused issues where a sufficiently slow computer would cause BHR to flood the STS with a large number of slow-running runnables.

(P.S. Both BHMgr Monitor, and BHMgr Processor should not appear outside of nightly builds, they won't even appear for beta users, so they're probably not super high priority)