Skip to content

fix: Fuse oneshot channel #147

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.

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
merged 4 commits into from
Mar 11, 2025
Merged

fix: Fuse oneshot channel #147

merged 4 commits into from
Mar 11, 2025

Conversation

rklaehn
Copy link
Collaborator

@rklaehn rklaehn commented Mar 11, 2025

It seems that under some weird circumstances we get a situation where an oneshot channel is polled after completion. Oneshot channel is not fused for no good reason that I can see, so we get a panic.

This did not happen before, but now it happens more frequently and causes frequent test failures in iroh-blobs CI.

There is a way to detect an already terminated oneshot channel in tokio 1.44 from 2025-03-07, so we could just modify the poll fn of the wrapper to call that. But since I don't want to update the tokio dep for a patch, I just wrap it in a fuse. We can do the is_terminated thing once we get tokio 1.44.

rklaehn added 4 commits March 10, 2025 15:27
tokio::select! requires the futures to be fused, see for example tokio-rs/tokio#3812
we don't want to update tokio just yet
@rklaehn rklaehn added the bug Something isn't working label Mar 11, 2025
@rklaehn rklaehn self-assigned this Mar 11, 2025
@n0bot n0bot bot added this to iroh Mar 11, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Mar 11, 2025
@@ -449,12 +449,13 @@ impl<C: ConnectionErrors> fmt::Display for RpcServerError<C> {
impl<C: ConnectionErrors> error::Error for RpcServerError<C> {}

/// Take an oneshot receiver and just return Pending the underlying future returns `Err(oneshot::Canceled)`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Take an oneshot receiver and just return Pending the underlying future returns `Err(oneshot::Canceled)`
/// Take an oneshot receiver and just return Pending when the underlying future returns `Err(oneshot::Canceled)`

@rklaehn
Copy link
Collaborator Author

rklaehn commented Mar 11, 2025

Checked in CI of n0-computer/iroh-blobs#68 that this indeed fixes this particular issue

@rklaehn rklaehn merged commit 03ad748 into main Mar 11, 2025
14 of 15 checks passed
@rklaehn rklaehn deleted the fuse-oneshot-channel branch March 11, 2025 08:24
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants