Skip to content

Commit

Permalink
task: add track_caller to public APIs (#4848)
Browse files Browse the repository at this point in the history
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 all the public APIs in the task
module of the tokio crate where the documentation describes how the
function may panic due to incorrect context or inputs.

In cases where `#[track_caller]` does not work, it has been left out.
For example, it currently does not work on async functions, blocks, or
closures. So any call stack that passes through one of these before
reaching the actual panic is not able to show the calling site outside
of tokio as the panic location.

The following functions have call stacks that pass through closures:
* `task::block_in_place`
* `task::local::spawn_local`

Tests are included to cover each potentially panicking function.

The following functions already had `#[track_caller]` applied everywhere
it was needed and only tests have been added:
* `task::spawn`
* `task::LocalKey::sync_scope`

Refs: #4413
  • Loading branch information
hds committed Jul 20, 2022
1 parent 56be528 commit 5288e1e
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 0 deletions.
1 change: 1 addition & 0 deletions tokio/src/runtime/mod.rs
Expand Up @@ -475,6 +475,7 @@ cfg_rt! {
/// ```
///
/// [handle]: fn@Handle::block_on
#[track_caller]
pub fn block_on<F: Future>(&self, future: F) -> F::Output {
#[cfg(all(tokio_unstable, feature = "tracing"))]
let future = crate::util::trace::task(future, "block_on", None, task::Id::next().as_u64());
Expand Down
1 change: 1 addition & 0 deletions tokio/src/task/local.rs
Expand Up @@ -487,6 +487,7 @@ impl LocalSet {
/// [`Runtime::block_on`]: method@crate::runtime::Runtime::block_on
/// [in-place blocking]: fn@crate::task::block_in_place
/// [`spawn_blocking`]: fn@crate::task::spawn_blocking
#[track_caller]
#[cfg(feature = "rt")]
#[cfg_attr(docsrs, doc(cfg(feature = "rt")))]
pub fn block_on<F>(&self, rt: &crate::runtime::Runtime, future: F) -> F::Output
Expand Down
2 changes: 2 additions & 0 deletions tokio/src/task/task_local.rs
Expand Up @@ -287,6 +287,7 @@ impl<T: Copy + 'static> LocalKey<T> {
/// # Panics
///
/// This function will panic if the task local doesn't have a value set.
#[track_caller]
pub fn get(&'static self) -> T {
self.with(|v| *v)
}
Expand Down Expand Up @@ -425,6 +426,7 @@ enum ScopeInnerErr {
}

impl ScopeInnerErr {
#[track_caller]
fn panic(&self) -> ! {
match self {
Self::BorrowError => panic!("cannot enter a task-local scope while the task-local storage is borrowed"),
Expand Down
92 changes: 92 additions & 0 deletions tokio/tests/task_panic.rs
@@ -0,0 +1,92 @@
#![warn(rust_2018_idioms)]
#![cfg(all(feature = "full", not(target_os = "wasi")))]

use futures::future;
use std::error::Error;
use tokio::{runtime::Builder, spawn, task};

mod support {
pub mod panic;
}
use support::panic::test_panic;

#[test]
fn local_set_block_on_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let rt = Builder::new_current_thread().enable_all().build().unwrap();
let local = task::LocalSet::new();

rt.block_on(async {
local.block_on(&rt, future::pending::<()>());
});
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn spawn_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
spawn(future::pending::<()>());
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn local_key_sync_scope_panic_caller() -> Result<(), Box<dyn Error>> {
tokio::task_local! {
static NUMBER: u32;
}

let panic_location_file = test_panic(|| {
NUMBER.sync_scope(1, || {
NUMBER.with(|_| {
let _ = NUMBER.sync_scope(1, || {});
});
});
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn local_key_with_panic_caller() -> Result<(), Box<dyn Error>> {
tokio::task_local! {
static NUMBER: u32;
}

let panic_location_file = test_panic(|| {
NUMBER.with(|_| {});
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn local_key_get_panic_caller() -> Result<(), Box<dyn Error>> {
tokio::task_local! {
static NUMBER: u32;
}

let panic_location_file = test_panic(|| {
NUMBER.get();
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

0 comments on commit 5288e1e

Please sign in to comment.