Skip to content

Commit 91682c8

Browse files
committed
time: add track_caller to public APIs
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: tokio-rs#4413
1 parent 55078ff commit 91682c8

File tree

4 files changed

+134
-0
lines changed

4 files changed

+134
-0
lines changed

tokio/src/time/clock.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ cfg_test_util! {
9696
///
9797
/// [`Sleep`]: crate::time::Sleep
9898
/// [`advance`]: crate::time::advance
99+
#[track_caller]
99100
pub fn pause() {
100101
let clock = clock().expect("time cannot be frozen from outside the Tokio runtime");
101102
clock.pause();
@@ -110,6 +111,7 @@ cfg_test_util! {
110111
///
111112
/// Panics if time is not frozen or if called from outside of the Tokio
112113
/// runtime.
114+
#[track_caller]
113115
pub fn resume() {
114116
let clock = clock().expect("time cannot be frozen from outside the Tokio runtime");
115117
let mut inner = clock.inner.lock();
@@ -189,6 +191,7 @@ cfg_test_util! {
189191
clock
190192
}
191193

194+
#[track_caller]
192195
pub(crate) fn pause(&self) {
193196
let mut inner = self.inner.lock();
194197

@@ -208,6 +211,7 @@ cfg_test_util! {
208211
inner.unfrozen.is_none()
209212
}
210213

214+
#[track_caller]
211215
pub(crate) fn advance(&self, duration: Duration) {
212216
let mut inner = self.inner.lock();
213217

tokio/src/time/driver/handle.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ cfg_rt! {
5353
///
5454
/// [`Builder::enable_time`]: crate::runtime::Builder::enable_time
5555
/// [`Builder::enable_all`]: crate::runtime::Builder::enable_all
56+
#[track_caller]
5657
pub(crate) fn current() -> Self {
5758
crate::runtime::context::time_handle()
5859
.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! {
8182
///
8283
/// [`Builder::enable_time`]: crate::runtime::Builder::enable_time
8384
/// [`Builder::enable_all`]: crate::runtime::Builder::enable_all
85+
#[track_caller]
8486
pub(crate) fn current() -> Self {
8587
panic!("{}", crate::util::error::CONTEXT_MISSING_ERROR)
8688
}

tokio/src/time/driver/sleep.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ cfg_not_trace! {
252252

253253
impl Sleep {
254254
#[cfg_attr(not(all(tokio_unstable, feature = "tracing")), allow(unused_variables))]
255+
#[track_caller]
255256
pub(crate) fn new_timeout(
256257
deadline: Instant,
257258
location: Option<&'static Location<'static>>,

tokio/tests/time_panic.rs

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
#![warn(rust_2018_idioms)]
2+
#![cfg(feature = "full")]
3+
4+
use futures::future;
5+
use parking_lot::{const_mutex, Mutex};
6+
use std::error::Error;
7+
use std::panic;
8+
use std::sync::Arc;
9+
use std::time::Duration;
10+
use tokio::runtime::{Builder, Runtime};
11+
use tokio::time::{self, interval, interval_at, timeout, Instant};
12+
13+
fn test_panic<Func: FnOnce() + panic::UnwindSafe>(func: Func) -> Option<String> {
14+
static PANIC_MUTEX: Mutex<()> = const_mutex(());
15+
16+
{
17+
let _guard = PANIC_MUTEX.lock();
18+
let panic_file: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));
19+
20+
let prev_hook = panic::take_hook();
21+
{
22+
let panic_file = panic_file.clone();
23+
panic::set_hook(Box::new(move |panic_info| {
24+
let panic_location = panic_info.location().unwrap();
25+
panic_file
26+
.lock()
27+
.clone_from(&Some(panic_location.file().to_string()));
28+
}));
29+
}
30+
31+
let result = panic::catch_unwind(func);
32+
// Return to the previously set panic hook (maybe default) so that we get nice error
33+
// messages in the tests.
34+
panic::set_hook(prev_hook);
35+
36+
if result.is_err() {
37+
panic_file.lock().clone()
38+
} else {
39+
None
40+
}
41+
}
42+
}
43+
44+
#[test]
45+
fn pause_panic_caller() -> Result<(), Box<dyn Error>> {
46+
let panic_location_file = test_panic(|| {
47+
let rt = basic();
48+
49+
rt.block_on(async {
50+
time::pause();
51+
time::pause();
52+
});
53+
});
54+
55+
// The panic location should be in this file
56+
assert_eq!(&panic_location_file.unwrap(), file!());
57+
58+
Ok(())
59+
}
60+
61+
#[test]
62+
fn resume_panic_caller() -> Result<(), Box<dyn Error>> {
63+
let panic_location_file = test_panic(|| {
64+
let rt = basic();
65+
66+
rt.block_on(async {
67+
time::resume();
68+
});
69+
});
70+
71+
// The panic location should be in this file
72+
assert_eq!(&panic_location_file.unwrap(), file!());
73+
74+
Ok(())
75+
}
76+
77+
#[test]
78+
fn interval_panic_caller() -> Result<(), Box<dyn Error>> {
79+
let panic_location_file = test_panic(|| {
80+
let _ = interval(Duration::from_millis(0));
81+
});
82+
83+
// The panic location should be in this file
84+
assert_eq!(&panic_location_file.unwrap(), file!());
85+
86+
Ok(())
87+
}
88+
89+
#[test]
90+
fn interval_at_panic_caller() -> Result<(), Box<dyn Error>> {
91+
let panic_location_file = test_panic(|| {
92+
let _ = interval_at(Instant::now(), Duration::from_millis(0));
93+
});
94+
95+
// The panic location should be in this file
96+
assert_eq!(&panic_location_file.unwrap(), file!());
97+
98+
Ok(())
99+
}
100+
101+
#[test]
102+
fn timeout_panic_caller() -> Result<(), Box<dyn Error>> {
103+
// let rt = Builder::new_current_thread().build().unwrap();
104+
// rt.block_on(async {
105+
// let _ = timeout(Duration::from_millis(5), future::pending::<()>());
106+
// });
107+
let panic_location_file = test_panic(|| {
108+
// Runtime without `enable_time` so it has no current timer set.
109+
let rt = Builder::new_current_thread().build().unwrap();
110+
rt.block_on(async {
111+
let _ = timeout(Duration::from_millis(5), future::pending::<()>());
112+
});
113+
});
114+
115+
// The panic location should be in this file
116+
assert_eq!(&panic_location_file.unwrap(), file!());
117+
118+
Ok(())
119+
}
120+
121+
122+
fn basic() -> Runtime {
123+
tokio::runtime::Builder::new_current_thread()
124+
.enable_all()
125+
.build()
126+
.unwrap()
127+
}

0 commit comments

Comments
 (0)