fix(core): wake idle workers when shutdown starts

[iltumio]

The bug

A Monitor with more than 30 workers never completes its shutdown if the extra workers are idle.

Monitor::run_all_workers awaits the workers with futures_util::future::join_all, which re-polls every future it holds on each wake-up only while there are 30 or fewer of them. Above that it switches to FuturesOrdered, which polls just the futures whose own waker was woken.

Starting a shutdown only flips the flag in ShutdownCtx and wakes the single waker stored by Shutdown's own Future impl. Nothing wakes the workers themselves. A worker parked on an idle backend — a cron worker waiting for a distant tick, an empty queue — is therefore never re-polled, never observes the flag, and the monitor never returns.

Both MonitorContext::shutdown() and Monitor::run_with_signal are affected. In production this means a process whose only remaining exit path is shutdown_timeout waits that deadline out on every single deploy, and any worker that would have finished its in-flight task in the meantime gets killed by the terminator instead.

The threshold is exact and reproducible on main, with no cron backend involved — the added test uses an empty MemoryStorage:

Idle workers	Result
30	shuts down in ~100 ms
31	hangs

It reproduces identically on 0.7.4, 1.0.0-rc.9 and the published 1.0.0-rc.10, so it is long-standing rather than a recent regression. The rc.10 check used the published apalis-core 1.0.0-rc.10 and apalis-cron 1.0.0-rc.9 crates, with idle cron workers on a monthly schedule, a signal after 100 ms and a 3 s outer timeout:

Idle cron workers	rc.10 as published	rc.10 + this PR
30	~102 ms	~102 ms
31	timed out at 3 s	~106 ms
51	timed out at 3 s	~103 ms
200	—	~106 ms

The fix

Three pieces, following the review discussion:

• run_all_workers awaits the workers with FuturesUnordered instead of join_all. This removes the accidental "re-poll everything" fast path join_all has for 30 or fewer futures, so behaviour no longer depends on the worker count. On its own this would make every idle monitor hang (the regression test fails at 1 worker with FuturesUnordered and no wake), which is why the next two pieces are required.
• MonitorContext::shutdown sets the flag and then calls WorkerContext::wake on every worker. This is the only place with the wake logic.
• run_with_signal routes the signal through self.context().shutdown() instead of Shutdown::shutdown_after, so the signal path — including the terminator branch — wakes the workers too. Without this the run_with_signal regression test hangs, and that is the SIGTERM path in production.

When every worker completes on its own, run_all_workers still calls start_shutdown() as before.

Graceful semantics are unchanged: waking a worker only causes it to be re-polled. It still drains its in-flight tasks — CallAll returns Pending while its queue is non-empty, and WorkerHandle only resolves at task_count == 0.

History: the first commit registered worker waker slots with Shutdown; the second woke the workers from run_all_workers by watching the handle; the third is the current shape.

Tests

Two regression tests in apalis-core/src/monitor/mod.rs, both of which hang without the change:

• shutdown_wakes_idle_workers — 31 idle workers, shut down via MonitorContext::shutdown().
• signal_shutdown_drains_active_work_with_many_idle_workers — 40 idle workers plus one busy worker, shut down via run_with_signal 50 ms into a 300 ms task; asserts the task completed before the monitor exited.

cargo test --all passes (132 tests). cargo fmt --all --check is clean. cargo check --all is clean, both with --all-features and with --no-default-features. cargo clippy --all --all-targets adds no warnings; its four existing warnings are in files this PR does not touch.

🤖 Generated with Claude Code

[geofmureithi]

This should be resolved in rc.10 released today, please confirm that first.

[geofmureithi]

Also you dont need to do all this, WorkerContext::wake should work

[iltumio]

You were right, thanks. 567d7b7 drops the waker registry from Shutdown and WorkerContext. run_all_workers now calls WorkerContext::wake on each worker once the shutdown handle resolves, and the change is confined to that function. Both regression tests still fail without the change and pass with it.

[iltumio]

I checked this against the published 1.0.0-rc.10 before changing anything, and the bug is still there. apalis-core 1.0.0-rc.10's src/ is identical to main, and start_shutdown() still only sets the flag and wakes the Shutdown future's own waker.

Probe: idle CronScheduler workers on a monthly schedule (apalis-core =1.0.0-rc.10, apalis-cron =1.0.0-rc.9), shut down with run_with_signal after 100 ms, inside a 3 s outer timeout:

Idle workers	rc.10 as published	rc.10 + this PR
30	~102 ms	~102 ms
31	timed out at 3 s	~106 ms
51	timed out at 3 s	~103 ms

I've also simplified the fix along the lines of your review comment, and updated the description to match.

[geofmureithi]

All you need is this:

 pub fn shutdown(&self) -> Result<(), MonitorError> {
        self.shutdown.start_shutdown();
        let _: Vec<_> = self.workers().iter().map(|s| s.wake()).collect();
        Ok(())
}

You dont need all that logic, Claude is just adding a lot of nonsense.

[geofmureithi]

Ok, I have seen the new changes. I agree the bug exists, I think it has been reported before. The solution is to add worker.wake() in the monitor_ctx.shutdown() method. Let me know if that works

[geofmureithi]

@iltumio I think the key things to note:

1. join_all is one part of the problem. I was not aware of the 30 Futures limit, we can resolve that with FuturesUnordered
2. The extra logic to wake workers on shutdown should be added to MonitorContext::shutdown
3. If 1&2 are applied, then we dont need to call wake in run_workers. This was the original intention, since a worker can internally stop.

This means:

• If all workers complete without a shutdown being called, start_shutdown is called
• If MonitorContext::shutdown is called, the we call wake on each worker, which should start the shutdown process

Let me know if that makes sense

[iltumio]

@iltumio I think the key things to note:

1. `join_all` is one part of the problem. I was not aware of the 30 Futures limit, we can resolve that with `FuturesUnordered`

2. The extra logic to wake workers on shutdown should be added to `MonitorContext::shutdown`

3. If 1&2 are applied, then we dont need to call `wake` in `run_workers`. This was the original intention, since a worker can internally stop.

This means:

* If all workers complete without a shutdown being called, `start_shutdown` is called

* If `MonitorContext::shutdown` is called, the we call wake on each worker, which should start the shutdown process

Let me know if that makes sense

First of all, I'm glad to see this project is still behind humans with critical thinking. Good job!

Unfortunately nowadays I don't have time to spend on coding directly, therefore I'm sorry that this pr looks like a back and forth between you and Claude :)

This is from Fable 5.1:

Makes sense overall, with two corrections from testing it:

1. FuturesUnordered doesn't lift the 30 limit — it removes the fast path. Below 30, join_all re-polls every future on each wake-up, which is the only reason small monitors work today; FuturesUnordered polls only futures whose waker fired, same as FuturesOrdered above 30. With FuturesUnordered and no explicit wake, the idle-worker test hangs at 1 worker, not 31. So I agree on using it (it makes the behaviour deterministic), but only together with a guaranteed wake.
2. Waking in MonitorContext::shutdown alone misses run_with_signal: shutdown_after calls Shutdown::start_shutdown() directly, never going through the context. With your proposal applied verbatim, the run_with_signal regression test still hangs — and that's the SIGTERM path in production.

Proposal that keeps your structure: FuturesUnordered in run_all_workers with no wake there; the wake loop lives only in MonitorContext::shutdown; and run_with_signal routes the signal through self.context().shutdown() instead of shutdown_after. Both tests pass, full suite green. Happy to push that if it matches what you had in mind.

[geofmureithi]

Proposal that keeps your structure: FuturesUnordered in run_all_workers with no wake there; the wake loop lives only in MonitorContext::shutdown; and run_with_signal routes the signal through self.context().shutdown() instead of shutdown_after. Both tests pass, full suite green. Happy to push that if it matches what you had in mind.

This is exactly what I was talking about!

[iltumio]

Makes sense overall — I tried it as described and hit two things worth flagging:

1. FuturesUnordered doesn't lift the 30 limit, it removes the fast path. Below 30, join_all re-polls every future on each wake-up, which is the only reason small monitors work today; FuturesUnordered polls only futures whose waker fired, same as FuturesOrdered above 30. With FuturesUnordered and no explicit wake, the idle-worker test hangs at 1 worker, not 31. So agreed on using it (behaviour becomes deterministic), but only together with a guaranteed wake.
2. Waking in MonitorContext::shutdown alone misses run_with_signal: shutdown_after calls Shutdown::start_shutdown() directly and never goes through the context. With the proposal applied verbatim, the run_with_signal regression test still hangs — and that's the SIGTERM path in production.

9500d7f keeps your structure and closes that gap: FuturesUnordered in run_all_workers with no wake there; the wake loop lives only in MonitorContext::shutdown; and run_with_signal routes the signal through self.context().shutdown() instead of shutdown_after. When all workers complete on their own, start_shutdown() is still called as before. Both regression tests pass, full suite green. Description updated to match.