Shard the HttpProxy shutdown Notify to cut lock contention - #969
Open
nbarbier-265 wants to merge 2 commits into
Open
Shard the HttpProxy shutdown Notify to cut lock contention#969nbarbier-265 wants to merge 2 commits into
nbarbier-265 wants to merge 2 commits into
Conversation
Every request that parks in read_request() registers a shutdown waiter on a single Notify shared by the whole proxy, and unregisters it when the read completes. Both operations serialize on the Notify's internal mutex, which becomes the scaling bottleneck on many-core machines: cloudflare#844 measured ~65% of off-CPU time in futex waits on a 128-core NUMA host. Shard the Notify by worker thread so waiter registration stays on a core-local, cache-line-padded shard. http_cleanup() notifies all shards. Streams whose headers are already buffered never touch the Notify at all because the biased select polls read_request() first. This also closes a lost-wakeup race: handle_new_request() never checked shutdown_flag, so a notify_waiters() that fired between read_request() returning Pending and the waiter registering was missed and the connection lingered until the grace period expired. The shutdown arm now registers the waiter first, then checks shutdown_flag before awaiting. Fixes cloudflare#844
h2 0.3.27 comes in through the same legacy aws chain as the existing rustls-webpki ignores: dial9-tokio-telemetry -> aws-sdk-s3-transfer-manager -> aws-config -> aws-smithy-http-client, which still uses hyper 0.14. The advisory's only fix is h2 >= 0.4.16 and no 0.3.x patch exists, so this cannot be resolved from this workspace's manifests. The vulnerable code needs a malicious HTTP/2 peer; in this chain h2 is only a TLS client to AWS endpoints. Every CI run has failed the cargo audit step since the advisory was published on 2026-08-17.
nbarbier-265
force-pushed
the
shard-shutdown-notify
branch
from
August 20, 2026 21:25
1484ea6 to
da7ac62
Compare
andrewhavck
added
enhancement
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
3 participants
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.
Fixes #844.
Every request that parks in
read_request()registers a shutdown waiter on a singleNotifyshared by the whole proxy, and unregisters it when the read completes. Both operations serialize on theNotify's internal mutex, which becomes the scaling bottleneck on many-core machines: #844 measured ~65% of off-CPU time in futex waits on a 128-core NUMA host.This change shards the
Notifyby worker thread (cache-line padded, power-of-two count fromavailable_parallelism, capped at 256). Waiter registration stays on a core-local shard;http_cleanup()notifies all shards. Streams whose headers are already buffered (h2 streams, pipelined h1) never touch theNotifyat all because the biased select pollsread_request()first.It also closes a pre-existing lost-wakeup race:
handle_new_request()never checkedshutdown_flag, so anotify_waiters()that fired betweenread_request()returning Pending and the waiter registering was missed, and the connection lingered until the grace period expired. The shutdown arm now registers the waiter first (Notified::enable), then checksshutdown_flagbefore awaiting. The two lock the same shard mutex, so either the flag load sees the store or the waiter receives the notification.Semantics are otherwise unchanged: connections parked in
read_request()are still woken immediately on shutdown, and in-flight requests are never aborted.Two tests added:
shutdown_wakes_parked_read_requests: parked keep-alive reads across multiple worker threads all wake onhttp_cleanup(). This is the behavior the flag-only fix proposed inNotify-based shutdown inHttpProxycauses severe lock contention on multi-core / NUMA systems #844 would break.shutdown_before_read_request_parks_returns_immediately: regression test for the lost-wakeup race. It hangs on current main.