mirror of
https://github.com/tokio-rs/tokio.git
synced 2025-09-25 12:00:35 +00:00
time: add track_caller to public APIs (#4791)
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 non-unstable public APIs in tokio-util 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 public functions that could not have `#[track_caller]` added for this reason are: * `time::advance` Tests are included to cover each potentially panicking function. In the following cases, `#[track_caller]` had already been added, and only tests have been added: * `time::interval` * `time::interval_at` Refs: #4413
This commit is contained in:
parent
ad412a9833
commit
3cc616877d
@ -96,6 +96,7 @@ cfg_test_util! {
|
||||
///
|
||||
/// [`Sleep`]: crate::time::Sleep
|
||||
/// [`advance`]: crate::time::advance
|
||||
#[track_caller]
|
||||
pub fn pause() {
|
||||
let clock = clock().expect("time cannot be frozen from outside the Tokio runtime");
|
||||
clock.pause();
|
||||
@ -110,6 +111,7 @@ cfg_test_util! {
|
||||
///
|
||||
/// Panics if time is not frozen or if called from outside of the Tokio
|
||||
/// runtime.
|
||||
#[track_caller]
|
||||
pub fn resume() {
|
||||
let clock = clock().expect("time cannot be frozen from outside the Tokio runtime");
|
||||
let mut inner = clock.inner.lock();
|
||||
@ -189,6 +191,7 @@ cfg_test_util! {
|
||||
clock
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
pub(crate) fn pause(&self) {
|
||||
let mut inner = self.inner.lock();
|
||||
|
||||
@ -208,6 +211,7 @@ cfg_test_util! {
|
||||
inner.unfrozen.is_none()
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
pub(crate) fn advance(&self, duration: Duration) {
|
||||
let mut inner = self.inner.lock();
|
||||
|
||||
|
@ -53,6 +53,7 @@ cfg_rt! {
|
||||
///
|
||||
/// [`Builder::enable_time`]: crate::runtime::Builder::enable_time
|
||||
/// [`Builder::enable_all`]: crate::runtime::Builder::enable_all
|
||||
#[track_caller]
|
||||
pub(crate) fn current() -> Self {
|
||||
crate::runtime::context::time_handle()
|
||||
.expect("A Tokio 1.x context was found, but timers are disabled. Call `enable_time` on the runtime builder to enable timers.")
|
||||
@ -81,6 +82,7 @@ cfg_not_rt! {
|
||||
///
|
||||
/// [`Builder::enable_time`]: crate::runtime::Builder::enable_time
|
||||
/// [`Builder::enable_all`]: crate::runtime::Builder::enable_all
|
||||
#[track_caller]
|
||||
pub(crate) fn current() -> Self {
|
||||
panic!("{}", crate::util::error::CONTEXT_MISSING_ERROR)
|
||||
}
|
||||
|
@ -252,6 +252,7 @@ cfg_not_trace! {
|
||||
|
||||
impl Sleep {
|
||||
#[cfg_attr(not(all(tokio_unstable, feature = "tracing")), allow(unused_variables))]
|
||||
#[track_caller]
|
||||
pub(crate) fn new_timeout(
|
||||
deadline: Instant,
|
||||
location: Option<&'static Location<'static>>,
|
||||
|
122
tokio/tests/time_panic.rs
Normal file
122
tokio/tests/time_panic.rs
Normal file
@ -0,0 +1,122 @@
|
||||
#![warn(rust_2018_idioms)]
|
||||
#![cfg(feature = "full")]
|
||||
|
||||
use futures::future;
|
||||
use parking_lot::{const_mutex, Mutex};
|
||||
use std::error::Error;
|
||||
use std::panic;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::runtime::{Builder, Runtime};
|
||||
use tokio::time::{self, interval, interval_at, timeout, Instant};
|
||||
|
||||
fn test_panic<Func: FnOnce() + panic::UnwindSafe>(func: Func) -> Option<String> {
|
||||
static PANIC_MUTEX: Mutex<()> = const_mutex(());
|
||||
|
||||
{
|
||||
let _guard = PANIC_MUTEX.lock();
|
||||
let panic_file: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));
|
||||
|
||||
let prev_hook = panic::take_hook();
|
||||
{
|
||||
let panic_file = panic_file.clone();
|
||||
panic::set_hook(Box::new(move |panic_info| {
|
||||
let panic_location = panic_info.location().unwrap();
|
||||
panic_file
|
||||
.lock()
|
||||
.clone_from(&Some(panic_location.file().to_string()));
|
||||
}));
|
||||
}
|
||||
|
||||
let result = panic::catch_unwind(func);
|
||||
// Return to the previously set panic hook (maybe default) so that we get nice error
|
||||
// messages in the tests.
|
||||
panic::set_hook(prev_hook);
|
||||
|
||||
if result.is_err() {
|
||||
panic_file.lock().clone()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pause_panic_caller() -> Result<(), Box<dyn Error>> {
|
||||
let panic_location_file = test_panic(|| {
|
||||
let rt = basic();
|
||||
|
||||
rt.block_on(async {
|
||||
time::pause();
|
||||
time::pause();
|
||||
});
|
||||
});
|
||||
|
||||
// The panic location should be in this file
|
||||
assert_eq!(&panic_location_file.unwrap(), file!());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_panic_caller() -> Result<(), Box<dyn Error>> {
|
||||
let panic_location_file = test_panic(|| {
|
||||
let rt = basic();
|
||||
|
||||
rt.block_on(async {
|
||||
time::resume();
|
||||
});
|
||||
});
|
||||
|
||||
// The panic location should be in this file
|
||||
assert_eq!(&panic_location_file.unwrap(), file!());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interval_panic_caller() -> Result<(), Box<dyn Error>> {
|
||||
let panic_location_file = test_panic(|| {
|
||||
let _ = interval(Duration::from_millis(0));
|
||||
});
|
||||
|
||||
// The panic location should be in this file
|
||||
assert_eq!(&panic_location_file.unwrap(), file!());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interval_at_panic_caller() -> Result<(), Box<dyn Error>> {
|
||||
let panic_location_file = test_panic(|| {
|
||||
let _ = interval_at(Instant::now(), Duration::from_millis(0));
|
||||
});
|
||||
|
||||
// The panic location should be in this file
|
||||
assert_eq!(&panic_location_file.unwrap(), file!());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn timeout_panic_caller() -> Result<(), Box<dyn Error>> {
|
||||
let panic_location_file = test_panic(|| {
|
||||
// Runtime without `enable_time` so it has no current timer set.
|
||||
let rt = Builder::new_current_thread().build().unwrap();
|
||||
rt.block_on(async {
|
||||
let _ = timeout(Duration::from_millis(5), future::pending::<()>());
|
||||
});
|
||||
});
|
||||
|
||||
// The panic location should be in this file
|
||||
assert_eq!(&panic_location_file.unwrap(), file!());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn basic() -> Runtime {
|
||||
tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.unwrap()
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user