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

JoinHandle destructor should not panic when dropping output #4412

Closed
Darksonn opened this issue Jan 19, 2022 · 0 comments · Fixed by #4430
Closed

JoinHandle destructor should not panic when dropping output #4412

Darksonn opened this issue Jan 19, 2022 · 0 comments · Fixed by #4430
Labels
A-tokio Area: The main tokio crate M-task Module: tokio/task

Comments

@Darksonn
Copy link
Contributor

Darksonn commented Jan 19, 2022

In Tokio, panics are generally caught and not propagated to the user when dropping the JoinHandle, however when dropping the JoinHandle of a task that has already completed, that panic can propagate to the user who dropped the JoinHandle. That happens here:

pub(super) fn drop_join_handle_slow(self) {
let mut maybe_panic = None;
// Try to unset `JOIN_INTEREST`. This must be done as a first step in
// case the task concurrently completed.
if self.header().state.unset_join_interested().is_err() {
// It is our responsibility to drop the output. This is critical as
// the task output may not be `Send` and as such must remain with
// the scheduler or `JoinHandle`. i.e. if the output remains in the
// task structure until the task is deallocated, it may be dropped
// by a Waker on any arbitrary thread.
let panic = panic::catch_unwind(panic::AssertUnwindSafe(|| {
self.core().stage.drop_future_or_output();
}));
if let Err(panic) = panic {
maybe_panic = Some(panic);
}
}
// Drop the `JoinHandle` reference, possibly deallocating the task
self.drop_reference();
if let Some(panic) = maybe_panic {
panic::resume_unwind(panic);
}
}

Note that the unset_join_interested call can only return an error if the task has already completed, so it is the output being dropped — not the future.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-task Module: tokio/task labels Jan 19, 2022
BraulioVM added a commit to BraulioVM/tokio that referenced this issue Jan 27, 2022
fixes tokio-rs#4412

From the issue description:

> In Tokio, panics are generally caught and not passed resumed when
dropping the JoinHandle, however when dropping the JoinHandle of a task
that has already completed, that panic can propagate to the user who
dropped the JoinHandle.

This PR fixes that.
BraulioVM added a commit to BraulioVM/tokio that referenced this issue Jan 27, 2022
fixes tokio-rs#4412

From the issue description:

> In Tokio, panics are generally caught and not passed resumed when
dropping the JoinHandle, however when dropping the JoinHandle of a task
that has already completed, that panic can propagate to the user who
dropped the JoinHandle.

This PR fixes that.
BraulioVM added a commit to BraulioVM/tokio that referenced this issue Jan 27, 2022
fixes tokio-rs#4412

From the issue description:

> In Tokio, panics are generally caught and not passed resumed when
dropping the JoinHandle, however when dropping the JoinHandle of a task
that has already completed, that panic can propagate to the user who
dropped the JoinHandle.

This PR fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-task Module: tokio/task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant