-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
provide a way to drop a runtime in an async context #2646
Conversation
Failing check: https://dev.azure.com/tokio-rs/Tokio/_build/results?buildId=6133&view=logs&j=edbb8bb1-bc6c-5b89-6b1a-b043f7e49efc Looks like it didn't even start building for some reason. Can someone please restart it? |
Oh wait, I misunderstood the UI. It did hang in test --features full apparently. |
I restarted the windows test. |
The question here might be how this fits together with whatever the future of structured concurrency is (#1879, #2596, one implementation in #2579 ). In such a model there shouldn't really be tasks that outlive the runtime anymore since the runtime represents a top level task scope. The runtime would wait in some kind of fashion (either synchronously/thread-blocking) or asynchronously for all tasks to complete. Could your application use a mechanism that sends a cancellation signal to the remainig tasks on the runtime and then waits for it asynchronously to complete, as described in the structured concurrency proposals? That wouldn't require to do a thread-blocking wait on the inner runtime anymore, but instead an |
For context, I created this after observing surprising (to me) behavior when Drop paniced in some tests I had initially wrote for #2645 . While it would be ideal of course to cancel things cleanly, right now in an async context where a clean cancellation path is unavailable (perhaps tearing things down after a panic or other failure?), our options are:
It seems a bit silly to me that an explicitly non-blocking shutdown should require a blocking context, hence this patch to help rectify this point of confusion. |
The change to the wait function is good. You are correct that we shouldn't "enter a blocking operation" if the operation doesn't actually block. Do you think it is worth adding |
@carllerche Having whether a function is at all usable in a particular context depend on a numeric parameter seemed a bit sketchy for me, but Id be okay with it either way I suppose if properly documented. |
My main hesitation w/ A more appropriate name could be |
Dropping a runtime normally involves waiting for any outstanding blocking tasks to complete. When this drop happens in an asynchronous context, we previously would issue a cryptic panic due to trying to block in an asynchronous context. This change improves the panic message, and adds a `shutdown_blocking()` function which can be used to shutdown a runtime without blocking at all, as an out for cases where this really is necessary.
e7fc2bd
to
56db531
Compare
@carllerche Changed to |
Motivation
Dropping a runtime normally involves waiting for any outstanding blocking tasks
to complete. When this drop happens in an asynchronous context, we previously
would issue a cryptic panic due to trying to block in an asynchronous context.
Solution
This change improves the panic message, and adds a
shutdown_now()
functionwhich can be used to shutdown a runtime without blocking at all, as an out for
cases where this really is necessary.