Repository navigation
feat(net): handle browser sign-in connections concurrently - #112
Merged
AlxcBTW merged 2 commits intoOct 27, 2026
Merged
Conversation
AlxcBTW
requested changes
Oct 27, 2026
AlxcBTW
left a comment
Member
There was a problem hiding this comment.
Thanks fago. Moving each connection to its own task fixes the stalled-connection block, and the test for it is good. Two things before merge:
- There is no bound on concurrent connections. Each task lives at most 5 seconds, but a local program can still open thousands in that time. Please add a small cap (a
tokio::sync::Semaphorewith about 16 permits,try_acquire_ownedbefore spawning; over the cap, just drop the stream) and a test that the callback still works while the cap is full of stalled connections. - When the sign-in finishes or times out,
servereturns but spawned handlers keep running up to 5 s with the stream. Please hold the tasks in aJoinSet(or abort them on return) so they end with the sign-in.
The accept-error backoff and the loopback check are kept, good. Minor: put use std::sync::Arc; with the other std import at the top.
Contributor
Author
|
Updated per review in commit 72875a3:
|
AlxcBTW
approved these changes
Oct 27, 2026
AlxcBTW
left a comment
Member
There was a problem hiding this comment.
Thanks fago, the cap, the abort on return and the test with the cap nearly full all look right. Merging once CI is green.
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.
What and why
Handles incoming browser sign-in connections concurrently in \crates/net/src/browser.rs.
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need.