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

Are kanal functions cancel-safe? #24

Closed
ghost opened this issue Nov 25, 2022 · 9 comments
Closed

Are kanal functions cancel-safe? #24

ghost opened this issue Nov 25, 2022 · 9 comments

Comments

@ghost
Copy link

ghost commented Nov 25, 2022

For example, is AsyncReceiver::recv cancel safe (e.g. safe to use with tokio::select)? Need some documentation corrections.

@ghost ghost changed the title Is kanal functions cancel-safe? Are kanal functions cancel-safe? Nov 25, 2022
@fereidani
Copy link
Owner

Could you please provide a definition of being cancel-safe?

@ghost
Copy link
Author

ghost commented Nov 28, 2022

Could you please provide a definition of being cancel-safe?

It's like atomicity: action will be fully performed or not performed at all if I cancel the future. I'm relying on the official tokio's documentation https://docs.rs/tokio/latest/tokio/macro.select.html#cancellation-safety

@fereidani
Copy link
Owner

recv is not cancel safe, safety here means there is a small possibility that received data gets dropped in the future drop if in the exact time that future is dropping another sender writes data(it is memory safe, drop safe), it is because there is no cancelation API for futures yet.
we can implement another cancel safe less performant method until then.

@ghost
Copy link
Author

ghost commented Dec 2, 2022

Cancelation-safety is the tokio-specific term, but I think this should be noted in the documentation. Tokio is the most popular runtime after all. Should I keep issue open until cancel-safe version is implemented? Will it be?
Thank you for the answer.

@fereidani
Copy link
Owner

Let's keep it open, I'm thinking of short-term and long-term solutions.
It surely should be documented in the right place of the project as the tokio is the most common async runtime.
The main source of the problem is the definition of the Future trait in rust language, it should have something like poll_try_cancel()->Poll<CancelationResult<Self::Output>> and CancelationResult could be:

enum CancelationResult<T>{
    Unimplemented,
    Done,
    Failed(T),
}

@al8n
Copy link

al8n commented Feb 23, 2023

Hi @fereidani, thanks for the amazing crate, may I ask about is there any plan to implement kanal's select? IMO, select is a common use case.

@fereidani
Copy link
Owner

@al8n please open a separate issue, we can discuss it there.

@thedodd
Copy link

thedodd commented Feb 27, 2024

recv is not cancel safe, safety here means there is a small possibility that received data gets dropped in the future drop if in the exact time that future is dropping another sender writes data(it is memory safe, drop safe), it is because there is no cancelation API for futures yet.

received data gets dropped in the future drop

@fereidani I think the semantics need to be clarified a bit here. What is "the future drop" in this context? Do you specifically mean: there is a small possibility that data gets dropped if data is sent over a channel at the exact time that the receiver is dropped?

Or perhaps you mean that data could be dropped specifically in the case that the sender is dropped before it has fully finished sending the data because the sending future is dropped?

Either way, thank you for the work on this crate. Really stoked about the benchmarks/performance possibilities here. I would love to adopt this crate for messaging in the data system I am building, which is otherwise entirely tokio channels. I just need to ensure that the data delivery guarantees are well-defined.

In the case of the system I am building, receiver channels will live for the duration of the program, with only a few cases where oneshot pools (my custom implementation) are used to amortize allocation costs.

As long as data is not going to be lost from within a normal use of tokio::select!{..} over long-lived channels, then this seems quite alright.

@fereidani
Copy link
Owner

Hey @thedodd, Just like other Rust polled futures, once you start polling, you need to continue until completion. Dropping midway can leave the future in an uncertain state. For instance, in async Rust when making an HTTP request, dropping the future after polling it once may leave you unaware of the request's result or status. To sum it up, ensure you use .await or poll the future correctly until the result is delivered. I'll close this issue to avoid further confusion, if rust adds a safe poll cancelation method which is unlikely, I will adjust the API to that.

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

No branches or pull requests

3 participants