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

task: add track_caller to block_in_place and spawn_local #5034

Merged
merged 2 commits into from Sep 20, 2022

Conversation

hds
Copy link
Contributor

@hds hds commented Sep 20, 2022

Motivation

Most of the public APIs in the task module were covered with the #[track_caller]
attribute in #4848. However, two functions were skipped because the call stack
leading to the panic passed through a closure (rust-lang/rust#87417).

The fact that this is missing was brought up again in #5030. At that point, a work around
solution was collectively worked out for that use case, bringing the panic call outside the
closure.

Solution

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source.

This change adds #[track_caller] to two public APIs in tokio task module which weren't added in #4848.

  • tokio::task::block_in_place
  • tokio::task::spawn_local

These APIs had call stacks that went through closures, which is a use case not supported by #[track_caller]. These two cases have been refactored so that the panic! call is no longer inside a closure.

Tests have been added for these two new cases.

Refs: #4413

@github-actions github-actions bot added the R-loom Run loom tests on this PR label Sep 20, 2022
Functions that may panic can be annotated with `#[track_caller]` so that
in the event of a panic, the function where the user called the
panicking function is shown instead of the file and line within Tokio
source.

This change adds `#[track_caller]` to two public APIs in tokio task
module which weren't added in #4848.
* `tokio::task::block_in_place`
* `tokio::task::spawn_local`

These APIs had call stacks that went through closures, which is a use
case not supported by `#[track_caller]`. These two cases have been
refactored so that the `panic!` call is no longer inside a closure.

Tests have been added for these two new cases.

Refs: #4413
@hds hds force-pushed the hds/task-track-caller-closures branch from 926de88 to 0e109c2 Compare September 20, 2022 12:37
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-task Module: tokio/task labels Sep 20, 2022
it doesn't work and shouldn't have been included in this PR.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, thank you!

@hds hds merged commit 7096a80 into master Sep 20, 2022
@hds hds deleted the hds/task-track-caller-closures branch September 20, 2022 20:55
dbischof90 pushed a commit to dbischof90/tokio that referenced this pull request Oct 1, 2022
…s#5034)

Functions that may panic can be annotated with `#[track_caller]` so that
in the event of a panic, the function where the user called the
panicking function is shown instead of the file and line within Tokio
source.

This change adds `#[track_caller]` to two public APIs in tokio task
module which weren't added in tokio-rs#4848.
* `tokio::task::block_in_place`
* `tokio::task::spawn_local`

These APIs had call stacks that went through closures, which is a use
case not supported by `#[track_caller]`. These two cases have been
refactored so that the `panic!` call is no longer inside a closure.

Tests have been added for these two new cases.

Refs: tokio-rs#4413
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 R-loom Run loom tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants