perf: push cancellation from Rust and register handler token sources - #82
Draft
jmbryan4 wants to merge 8 commits into
Draft
perf: push cancellation from Rust and register handler token sources#82jmbryan4 wants to merge 8 commits into
jmbryan4 wants to merge 8 commits into
Conversation
Replace the pull cancellation bridge with a push path. Rust spawns a watcher task per handler call. The watcher races on_cancel() against a stop signal and pushes cancel() to a C# callback. This removes the per-message TaskCompletionSource, Task.WhenAny, monitor task, and the CA2025 pragma. Pool the CancellationTokenSource per handler call. Each slot carries a rental epoch. A stale push checks the epoch first, so it cannot cancel a later rental. Rent() retires a slot that a stale push cancelled while it sat in the pool, so a handler never starts with a cancelled token. Deleted tests and their surviving stronger tests: - BridgeCancellationCancelsCtsWhenOnCancelCompletes -> HandleMessageCancelsTokenWhileHandlerIsRunning - BridgeCancellationDoesNotCancelCtsWhenHandlerCompletesFirst -> CancelPushAfterHandlerCompletedIsANoOp and StaleEpochCannotCancelReRentedSlot - BridgeCancellationSwallowsSynchronousOnCancelFault -> HandleMessageCompletesWhenWatchRegistrationFaults - BridgeCancellationSwallowsLateFaultWhenHandlerCompletesFirst is deleted without replacement. The hazard is gone: cancel() is a synchronous call from a Rust task, so no C# Task can fault late.
jmbryan4
marked this pull request as draft
August 5, 2026 05:22
Give every handler a unique ID. Hold the slot gate across the ID check, cancellation, and reset. This makes stale callbacks no-ops and keeps TryReset away from concurrent cancellation. Run cancellation in the Rust handler task. Remove the watcher task, callback trait, and fixed primitive pool capacity. Size each pool from client concurrency. Track regenerated bindings. StaleEpochCannotCancelReRentedSlot and StaleCancelWhilePooledDoesNotLeakACancelledToken now use StaleHandlerIdCannotCancelReRentedSlot. CancelledSourceIsDisposedNotPooled now uses CancelledSourceIsReplacedBeforeNextRental. PoolRetainsAtMostCapacity now uses PoolRetainsConfiguredCapacity. RentedSourceIsNeverCancelledUnderAnyOpSequence now uses StaleHandlerIdCannotCancelReRentedSlot and ReturnWaitsForCancellationBeforeReset. CancelPushAfterHandlerCompletedIsANoOp now uses CancelAfterHandlerCompletedIsANoOp. HandleMessageCompletesWhenWatchRegistrationFaults and HandleTimerCompletesWhenWatchStopFaults now use HandleMessageCancelsTokenWhileHandlerIsRunning and HandleTimerCancelsTokenWhileHandlerIsRunning.
Every build path runs bindgen, so the committed copy is never consumed. Restore the .gitignore entry and untrack ProsodyFfi.cs. Update the Definition of Done to match.
The generated shim starts handlers with Task.Run, so Rust can call cancel(handler_id) before the handler rents its cancellation slot. The bridge now probes should_cancel once after the rent and pre-cancels its own token, so an early cancel is never lost. CancelIfCurrent no longer cancels under the slot gate. It marks the source cancel-pending and queues the cancel to the thread pool, so user token callbacks never run on the Rust runtime thread or under the lock. Return retires a cancel-pending source instead of resetting it, so a late cancel cannot reach the next renter. Faults from token callbacks are logged, not thrown across the FFI. The FFI client now resolves max_concurrency once (options, then PROSODY_MAX_CONCURRENCY, then the scheduler default) and exposes it as max_concurrency(). The C# client sizes the cancellation pool from that value. This deletes the duplicated resolution in ClientOptions. Timer and message handlers share the same scheduler permits upstream, so the pool capacity equals the true concurrency bound. Also extract the duplicated select dance into CsHandler::invoke, remove the unused CsCheck package, and correct the false ordering comment. Deleted tests and their surviving stronger tests: - ReturnWaitsForCancellationBeforeReset and CancelledSourceIsReplacedBeforeNextRental are covered by CancelPendingSourceIsRetiredOnReturn and SynchronouslyCancelledSourceIsReplacedOnReturn. - ResolveMaxConcurrencyUsesConfiguredValue and ResolveMaxConcurrencyRejectsZero die with the deleted code; the native scheduler validates the bound.
The node reported its internal Docker IP to clients, so host-run tests failed after driver peer discovery. Broadcast 127.0.0.1 instead. The prosody repo compose file carries the same change.
Contributor
Author
Benchmark findingsThree mechanisms were benchmarked :
BenchmarkDotNet — one handler invocation, mechanism only (.NET 10, Arm64)
The benchmark simulates dotnet-counters — 32 concurrent workers, 25 s sustained load
Conclusions
|
…registry Benchmarks on PR #82 show the push-cancellation path carries the whole win. The pool saved only 96 B per invocation, ran slower than a fresh source per invocation, and serialized handlers on one gate under load. It also carried the slot-reuse race machinery and a cross-boundary concurrency invariant. Each invocation now registers a fresh CancellationTokenSource in a ConcurrentDictionary keyed by handler ID and removes it on completion. Sources are never reused, so a stale cancel can only reach an abandoned source. The native max_concurrency report and the pool-sizing plumbing are gone. Deleted tests and their surviving coverage: - CancellationTokenSourcePoolTests.CancelReachesTheActiveRenter -> CancellationRegistryTests.CancelReachesTheRegisteredHandler. - StaleHandlerIdCannotCancelReRentedSlot, CancelPendingSourceIsRetiredOnReturn, SynchronouslyCancelledSourceIsReplacedOnReturn -> LateCancelCannotReachALaterHandler and RegisterReturnsAFreshUncancelledSource; without reuse the stale-cancel class is structural, not defended. - ReturnedHandlerIdCanRentAgain -> CompletedHandlerIdCanRegisterAgain. - PoolRetainsConfiguredCapacity, PoolRejectsCapacityAboveTheAllocationLimit -> removed with the capacity invariant; SecondRegistrationForAnActiveHandlerThrows keeps the identity guard. - ClientOptionsValidatorTests.MaxConcurrencyOutsidePoolCapacityFails -> MaxConcurrencyOfZeroFails; the upper bound was pool-only.
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.
Experimentation on cancellation changes