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

Document future cancellation #2372

Open
bendk opened this issue Dec 31, 2024 · 5 comments
Open

Document future cancellation #2372

bendk opened this issue Dec 31, 2024 · 5 comments

Comments

@bendk
Copy link
Contributor

bendk commented Dec 31, 2024

Update https://mozilla.github.io/uniffi-rs/next/internals/async.html and add some discussion on cancelling futures/async functions.

@extraymond
Copy link

extraymond commented Jan 1, 2025

#2360 (comment)

Continue on my test case here.

After some tinkering, in contrasts to my original attempt to cancel future via KeyboardInterrupt in python, the more reliable way to cancel future seems to be using signal handler or some sort of global control to call the cancel function exposed to python, one candidate would be signal.add_signal_handler

    handle = RustHandle()
    signals = (signal.SIGHUP, signal.SIGTERM, signal.SIGINT)
    for s in signals:
        loop.add_signal_handler(
            s, lambda s=s: handle.shutdown(s, loop)
        )
    try:
        loop.run_until_complete(handle.entry())
    except asyncio.exceptions.CancelledError:
        logger.debug("shutdown via exit signal")

I think on the uniffi side, the current design is mostly fine, because mapping rust's async runtime behaviour, which is configurable, to any of the exposed languages adds complexity, so cancellation should really be provided by the author. Therefore if the event_loop is pulling stuff from the other side it can decide how to delegate to rust for the exposed cancel function.

Nonetheless, the current version of uniffi python helper code generated some error msg that is not easy to debug or understand, I think it's better to:

  1. improve the exception captured here
    @_UNIFFI_RUST_FUTURE_CONTINUATION_CALLBACK
    so that an author should implement something to cancel the future without relying on exception to cancel the task for you, at least it's a bit confusing for me the first time seeing it.
  2. don't silently capture the exception, it might be better to either: re-raise the original exception, or raise a special exception so that user-code can capture it and immediately call the cancel function exposed
while True:
  try:
    await RustFututure()
  except exception_during_uniffi:
   handle.cancel()

@dev-ardi
Copy link

dev-ardi commented Jan 2, 2025

I'm personally more keen on adding a dependency on tokio and starting every async ffi call as a tokio task to avoid every issue with cancellation and make it more reliable.

What we do is we have a static instance of a runtime and spawn all tasks there.

@bendk
Copy link
Contributor Author

bendk commented Jan 2, 2025

After some tinkering, in contrasts to my original attempt to cancel future via KeyboardInterrupt in python, the more reliable way to cancel future seems to be using signal handler or some sort of global control to call the cancel function exposed to python, one candidate would be signal.add_signal_handler

Another alternative is implementing cancellation to the interface exposed from the Rust side. You can add a cancel function there, that cancels the task using regular Rust mechanisms like tokio's CancellationToken. IMO, this is the simplest solution, would it work for you?

don't silently capture the exception, it might be better to either: re-raise the original exception, or raise a special exception so that user-code can capture it and immediately call the cancel function exposed

Neither of these is an option since this happens inside a C callback that Rust is calling -- there's no realistic way to propagate the Python exception up the stack there. However, I think it might be possible catch any exceptions and cancel/free the future in that case.

I'm personally more keen on adding a dependency on tokio and starting every async ffi call as a tokio task to avoid every issue with cancellation and make it more reliable.

There's actually some support right now for this with the async_runtime attribute. However, I don't think it's documented -- maybe because we aren't sure if the current implementation is the best way going forward. There's some discussion in #1726 about this.

@extraymond
Copy link

@bendk Thx for the reply

Another alternative is implementing cancellation to the interface exposed from the Rust side. You can add a cancel function there, that cancels the task using regular Rust mechanisms like tokio's CancellationToken. IMO, this is the simplest solution, would it work for you?

This occurs to be more reliable with the assumption that one might need to create and handle the async-runtime from the rust side. Functionally it's more than capable of conveying my idea through rust this way with the current implementation of uniffi.

One observation I found is that uniffi relied on the host language to run the event_loop where bringing another async-runtime is optional. In that mentality I think one would like to continue relying on the host language to provide cancellation in a way that it's native to them. In the case of python, exception and signal handler seems to be more common.

Neither of these is an option since this happens inside a C callback that Rust is calling -- there's no realistic way to propagate the Python exception up the stack there. However, I think it might be possible catch any exceptions and cancel/free the future in that case.

After some testing, I think it's not only related to async but any call passed through ctypes as well, it's not possible to be effected by runtime exception.

#[uniffi::export]
pub fn halting() {
    loop {

    }
}

And it'll not be captured anyhow

    try:
        halting()
    except Exception as e:
        print(e)

where the py-equivalent will play nicely with exception and exit

def py_halt():
    while True:
        pass

Would this be an good summary of what will happen with regards of an exception?

  1. if exception happens during _UNIFFI_RUST_FUTURE_CONTINUATION_CALLBACK call, it'll be captured without propagating back into python, I think the future just stopped being evaluated from the python side afterwards?
  2. if exception happens as a future result in a Result<T, E>, it'll be correctly mapped to an python exception and users are free to handle it
  3. if an future is lazy evaluated from host, such as tokio::spawn, when runtime exception happens(in other host's code) and the future is not ready, user still have a chance to call cancel if provided through uniffi, just don't block the main thread of the call stack from the rust call if it's long running

If my summary above is correct, I think it's best to advice authors to implement cancellation of any long running task from rust so that it'll be less surprise if users aren't that familiar with this mechanism.

From the host language author's perspective, I think a small chapter like this might be enough to keep them going. This is ranged from how much involvement they want to intervene from mostly rust to mostly runtime

// actively capture interruption
#[uniffi::export]
async fn rust_fn() {
  tokio::singla::ctrl_c().await;
}

// cooperative interruption
#[uniffi::export]
async fn  rust_fn(signal_for_cancel: u8) {
  // signal mask or some sort that host can passed to cancel the future from rust's side
}


// runtime exception
#[uniffi::export]
async fn runst_fn() {
  let token = CancellationToken::new();
  let handle = tokio::spawn(async move {
   select(token, other_future);
 });

 };

  // store the cancel token in another function so that user can call during runtime exception
});

}

There's actually some support right now for this with the async_runtime attribute. However, I don't think it's documented -- maybe because we aren't sure if the current implementation is the best way going forward. There's some discussion in #1726 about this.

This seems like an good default, I can see it working well as long as it can optionally toggle off.

@bendk
Copy link
Contributor Author

bendk commented Jan 3, 2025

One observation I found is that uniffi relied on the host language to run the event_loop where bringing another async-runtime is optional. In that mentality I think one would like to continue relying on the host language to provide cancellation in a way that it's native to them. In the case of python, exception and signal handler seems to be more common.

This is a good point, but the issue is that not all languages provider a nice way to cancel an async call -- at least not in a way that UniFFI can use. See the Swift discussion. That said, I'm not sure that Python falls into this category. Maybe we can support task cancellation (in fact, maybe we do already see the test below). However, I don't feel like KeyboardInterrupt is the right way to do this. I think Task.cancel would be more appropriate. What happens if you changed you code so that you called that method on the top-level task, maybe in a timeout or something? I think that's what this test does.

Would this be an good summary of what will happen with regards of an exception?

I would split this up a different way:

  • If you throw the error that's defined in the interface (the E half of Result<T, E>, then it will be returned to Rust normally.
  • If you throw another different error, then From<uniffi::UnexpectedUniFFICallbackError> will be used to convert it to the E type. (https://mozilla.github.io/uniffi-rs/next/foreign_traits.html talks about this some, maybe we could improve that section though).
  • However, KeyboardInterrupt is really a special case, since it causes the exception to happen in UniFFI code where we currently assume that no error will happen. We should probably improve our handling of this, can you open an issue for it?
  • Of course, the last case in an unexpected bug in the UniFFI code which is Undefined Behavior.
  • Sync/Async shouldn't change the fundamental behavior.

if an future is lazy evaluated from host, such as tokio::spawn, when runtime exception happens(in other host's code) and the future is not ready, user still have a chance to call cancel if provided through uniffi, just don't block the main thread of the call stack from the rust call if it's long running

I didn't quite follow this one, but I don't believe we make any guarantees like this.

From the host language author's perspective, I think a small chapter like this might be enough to keep them going. This is ranged from how much involvement they want to intervene from mostly rust to mostly runtime

Totally agree, hopefully I'll get around to that sometime relatively soon.

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