Skip to content

Bound bi-stream handler concurrency at accept site#477

Open
MorganaFuture wants to merge 2 commits into
superfly:mainfrom
MorganaFuture:MorganaFuture/bi-stream-concurrency-limit
Open

Bound bi-stream handler concurrency at accept site#477
MorganaFuture wants to merge 2 commits into
superfly:mainfrom
MorganaFuture:MorganaFuture/bi-stream-concurrency-limit

Conversation

@MorganaFuture

Copy link
Copy Markdown

A peer could open unbounded bi-streams and exhaust memory before the inner agent.limits().sync check inside serve_sync ever fired. Move the try_acquire up to spawn_bipayload_handler, before tokio::spawn, and write MaxConcurrencyReached on the stream when no permits remain.

The duplicate check inside serve_sync is removed: keeping both would consume two permits per sync from the same 3-permit pool.

A peer could open unbounded bi-streams and exhaust memory before the
inner `agent.limits().sync` check inside `serve_sync` ever fired. Move
the try_acquire up to `spawn_bipayload_handler`, before `tokio::spawn`,
and write `MaxConcurrencyReached` on the stream when no permits remain.

The duplicate check inside `serve_sync` is removed: keeping both would
consume two permits per sync from the same 3-permit pool.
// adding a parallel counter. Acquiring before `tokio::spawn`
// prevents an unbounded peer from exhausting tasks/memory just
// by opening streams.
let permit = match agent.limits().sync.clone().try_acquire_owned() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we'd want to make this configurable? Right now we have it set to three and there's no way to tune it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@MorganaFuture MorganaFuture requested a review from somtochiama May 18, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants