fix(clob): keep heartbeats alive across client clones - #92
Open
mattfaltyn wants to merge 1 commit into
Open
Conversation
mattfaltyn
marked this pull request as ready for review
July 19, 2026 19:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DroppingCancellationTokenowner is droppedClientclones, including order-builder clones, are consumed or droppedRoot cause
ClientandDroppingCancellationTokenboth deriveClone, but every cloned wrapper previously calledCancellationToken::cancel()fromDrop.OrderBuilderembeds a cloned client, so completing or dropping a normal order builder silently terminated the shared heartbeat task while the original client continued reportingheartbeats_active() == true.The fix uses
Arc::into_innerto atomically identify the final receiver owner. This preserves existing explicit-stop and multiple-owner synchronization behavior while ensuring exactly one final drop performs cancellation, including under concurrent drops.Impact
Heartbeat-enabled clients can build orders without silently stopping the dead-man heartbeat mechanism and risking order-priority loss or cancellation of open orders.
Validation
cargo build --all-targets --all-featurescargo test— 15 unit tests and 4 doctests passed; feature-gated integrations correctly skippedcargo +nightly-2025-11-24 fmt --all -- --checkcargo +1.88 clippy --all-targets --all-features -- -D warningscargo +1.88 clippy --all-targets -- -D warningscargo sort --checkwith cargo-sort 2.0.2cargo +1.88 llvm-cov --all-features --workspace --lcov --output-path lcov.info— 559 all-feature tests passedPRE_COMMIT_HOME=/tmp/pre-commit pre-commit run --files src/clob/client.rs tests/clob.rsFixes #91
Note
Medium Risk
Touches shared heartbeat teardown for all authenticated client clones; wrong refcount logic could leave heartbeats running or stop them too early, but scope is narrow and covered by a new regression test.
Overview
Fixes a bug where dropping any cloned
Client(including a temporary limit-order builder) cancelled the shared heartbeat task while the main client still reported heartbeats as active.DroppingCancellationToken’sDropnow callsCancellationToken::cancel()only when the dropped instance holds the lastArcto the cleanup receiver (Arc::into_inner), so intermediate clones fromlimit_order()/market_order()no longer stop the background heartbeat loop. Explicitstop_heartbeats/cancel_and_waitbehavior is unchanged.Adds a heartbeats integration test that drops a limit-order builder, asserts heartbeats keep posting, then verifies
stop_heartbeatsstill shuts them down.Reviewed by Cursor Bugbot for commit 03fa66a. Bugbot is set up for automated code reviews on this repo. Configure here.