fix(consensus): wait for SIGTERM shutdown task - #380
Conversation
|
Reviewed Why this fixes the reported raceThe important ordering in
On the PR branch, It also preserves the existing Test coverage caveat
That is not a reason to reject the implementation — the ordering is visible and correct in the source — but it means the new test would still pass if the signal wiring or the placement of Verification status
One small implementation note: Disclosure: I'm an external community contributor, not affiliated with Circle, with no write access to this repository. Advisory review only, not an approval. |
Summary
Coordinate
Node::run()with the SIGTERM shutdown task so the Tokio runtime cannot be dropped while graceful shutdown is still draining and persisting the savepoint.Problem
The SIGTERM handler runs in a spawned Tokio task. Stopping the Node actor can make the application task return cleanly, allowing
Node::run()to return before the SIGTERM task reachesdrain_before_exit()andexit(143).Fix
JoinHandle.Node::run()wait for that task instead of allowing the runtime to return early.Testing
cargo fmt --all— passed.git diff --check— no whitespace errors; Git emitted only the Windows LF/CRLF conversion warning.cargo test -p arc-node-consensus— could not complete on native Windows because thereth-providerdependency importsChangesetOffsetReader, which is gated behindcfg(all(feature = "std", unix))inreth_static_file_types.Fixes #360