Skip to content

test fix: keep connection alive #8458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bw-solana merged 1 commit into from
Oct 14, 2025
Merged

test fix: keep connection alive #8458

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion streamer/src/nonblocking/quic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,7 @@ pub mod test {

let SpawnTestServerResult {
join_handle,
receiver: _,
receiver,
Copy link
Copy Markdown

@steviez steviez Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch! I feel like I might have known this could happen at some point but didn't remember this time around. Had to dig a little to find this in the book:

Note that there is a subtle difference between using only _ and using a name that starts with an underscore. The syntax _x still binds the value to the variable, whereas _ doesn’t bind at all. To show a case where this distinction matters, Listing 19-21 will provide us with an error.
...
However, using the underscore by itself doesn’t ever bind to the value

Even then, I'm surprised the docs aren't a little more direct about this potential footgun. Some issues in rust-lang would suggest that other people find this bit a little tricky too but 🤷

FWIW, it looks like doing receiver: _receiver would have sufficed too but the explicit drop() at the end does the job as well !

Copy link
Copy Markdown
Author

@bw-solana bw-solana Oct 14, 2025

Choose a reason for hiding this comment

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

yeah, such a foot gun... I'm happy to change this to _receiver as opposed to the explicit drop at the end.

Copy link
Copy Markdown

@alexpyattaev alexpyattaev Oct 14, 2025

Choose a reason for hiding this comment

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

Explicit is better than implicit.

Copy link
Copy Markdown

@KirillLykov KirillLykov Oct 15, 2025

Choose a reason for hiding this comment

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

I think we also should not wait without timeout for some atomics to have desired value. Will fix this as well.

server_address,
stats,
cancel,
Expand Down Expand Up @@ -1957,6 +1957,11 @@ pub mod test {
assert!(start.elapsed().as_secs() < 1);

cancel.cancel();
// Explicitly drop receiver here so that it doesn't get implicitly
// dropped earlier. This is necessary to ensure the server stays alive
// and doesn't issue a cancel to kill the connection earlier than
// expected.
drop(receiver);
join_handle.await.unwrap();
}

Expand Down
Loading