Skip to content
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

Feature request: Allow non-Send broadcast values in more (generic) contexts #6902

Open
jplatte opened this issue Oct 14, 2024 · 3 comments · May be fixed by #6908
Open

Feature request: Allow non-Send broadcast values in more (generic) contexts #6902

jplatte opened this issue Oct 14, 2024 · 3 comments · May be fixed by #6908
Labels
A-tokio-stream Area: The tokio-stream crate A-tokio-util Area: The tokio-util crate C-feature-request Category: A feature request.

Comments

@jplatte
Copy link
Member

jplatte commented Oct 14, 2024

Is your feature request related to a problem? Please describe.

In my library eyeball-im, I use tokio_util::sync::ReusableBoxFuture to implement a Stream on top of a tokio::sync::broadcast channel. This is the same thing tokio_stream::wrappers::BroadcastStream does. However, this means that the messages being sent over the channel must be Send. I can also box the future myself without requiring Sendness of the message type, but then my own receive futures become non-Send.

Describe the solution you'd like

I would like to avoid boxing to let the compiler infer Send-ness of the involves futures. If tokio::sync::broadcast::Receiver::recv returned a named future, tokio_stream::wrappers::BroadcastStream could drop the Send bound on T, and I could drop the same bound in my library too.

Describe alternatives you've considered

I have a PR with a copy-pasted ReusableBoxFuture without the Send bounds, and a wrapper type for it that is only ever instantiated with the same (unnameable) future that invokes receiver.recv() and that I know is Send if the message type is. There's a static assertion that T: Send -> <unnameable future>: Send, and unsafely implemented Send for the wrapper type: jplatte/eyeball@e4fd1f5#diff-5a8e4292bd84885e0f16976040ee79e231ab74df5283d52565cd4fec4ab92d75R271-R292

However, it seems wrong to require unsafe for this. My code's soundness is probably relying on tokio not doing weird things with the Send-ness of the broadcast::Receiver::recv future.. not a huge deal but it doesn't feel good to release unsafe code with no serious attempt at a soundness proof.

@jplatte jplatte added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Oct 14, 2024
@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate A-tokio-stream Area: The tokio-stream crate and removed A-tokio Area: The main tokio crate labels Oct 15, 2024
@jplatte
Copy link
Member Author

jplatte commented Oct 15, 2024

@Darksonn I see you added some labels. Is this feature request is "accepted" then, i.e. a PR for this would be welcome?

@Darksonn
Copy link
Contributor

I don't know if adding labels necessarily means that, but yes I would be okay with a PR for this. Sorry for not responding earlier. I think we can perhaps add an unsafe method to ReusableBoxFuture that takes a Future without a 'static or Send bound, and then it's up to the caller to make sure that they don't use the future in violation of its true trait bounds.

@jplatte
Copy link
Member Author

jplatte commented Oct 16, 2024

I don't know if adding labels necessarily means that

Yeah I don't think it does, but it means you looked at and understood the request, so I felt like nothing more was going to happen until I explicitly asked :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-stream Area: The tokio-stream crate A-tokio-util Area: The tokio-util crate C-feature-request Category: A feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants