Skip to content

Commit 0e109c2

Browse files
committed
task: add track_caller to block_in_place and spawn_local
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
1 parent d69e5be commit 0e109c2

File tree

5 files changed

+52
-14
lines changed

5 files changed

+52
-14
lines changed

tokio/src/runtime/scheduler/multi_thread/worker.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ pub(super) fn create(
249249
(handle, launch)
250250
}
251251

252+
#[track_caller]
252253
pub(crate) fn block_in_place<F, R>(f: F) -> R
253254
where
254255
F: FnOnce() -> R,
@@ -275,7 +276,7 @@ where
275276

276277
let mut had_entered = false;
277278

278-
CURRENT.with(|maybe_cx| {
279+
let setup_result = CURRENT.with(|maybe_cx| {
279280
match (crate::runtime::enter::context(), maybe_cx.is_some()) {
280281
(EnterContext::Entered { .. }, true) => {
281282
// We are on a thread pool runtime thread, so we just need to
@@ -288,22 +289,24 @@ where
288289
// method:
289290
if allow_blocking {
290291
had_entered = true;
291-
return;
292+
return Ok(());
292293
} else {
293294
// This probably means we are on the current_thread runtime or in a
294295
// LocalSet, where it is _not_ okay to block.
295-
panic!("can call blocking only when running on the multi-threaded runtime");
296+
return Err(
297+
"can call blocking only when running on the multi-threaded runtime",
298+
);
296299
}
297300
}
298301
(EnterContext::NotEntered, true) => {
299302
// This is a nested call to block_in_place (we already exited).
300303
// All the necessary setup has already been done.
301-
return;
304+
return Ok(());
302305
}
303306
(EnterContext::NotEntered, false) => {
304307
// We are outside of the tokio runtime, so blocking is fine.
305308
// We can also skip all of the thread pool blocking setup steps.
306-
return;
309+
return Ok(());
307310
}
308311
}
309312

@@ -312,7 +315,7 @@ where
312315
// Get the worker core. If none is set, then blocking is fine!
313316
let core = match cx.core.borrow_mut().take() {
314317
Some(core) => core,
315-
None => return,
318+
None => return Ok(()),
316319
};
317320

318321
// The parker should be set here
@@ -331,8 +334,13 @@ where
331334
// steal the core back.
332335
let worker = cx.worker.clone();
333336
runtime::spawn_blocking(move || run(worker));
337+
Ok(())
334338
});
335339

340+
if let Err(panic_message) = setup_result {
341+
panic!("{}", panic_message);
342+
}
343+
336344
if had_entered {
337345
// Unset the current task's budget. Blocking sections are not
338346
// constrained by task budgets.

tokio/src/sync/mpsc/bounded.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ impl<T> Sender<T> {
633633
/// }
634634
/// }
635635
/// ```
636+
#[track_caller]
636637
#[cfg(feature = "time")]
637638
#[cfg_attr(docsrs, doc(cfg(feature = "time")))]
638639
pub async fn send_timeout(

tokio/src/task/blocking.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ cfg_rt_multi_thread! {
7070
/// This function panics if called from a [`current_thread`] runtime.
7171
///
7272
/// [`current_thread`]: fn@crate::runtime::Builder::new_current_thread
73+
#[track_caller]
7374
pub fn block_in_place<F, R>(f: F) -> R
7475
where
7576
F: FnOnce() -> R,

tokio/src/task/local.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,10 @@ cfg_rt! {
314314
where F: Future + 'static,
315315
F::Output: 'static
316316
{
317-
CURRENT.with(|maybe_cx| {
318-
match maybe_cx.get() {
319-
None => panic!("`spawn_local` called from outside of a `task::LocalSet`"),
320-
Some(cx) => cx.spawn(future, name)
321-
}
322-
})
317+
match CURRENT.with(|maybe_cx| maybe_cx.get()) {
318+
None => panic!("`spawn_local` called from outside of a `task::LocalSet`"),
319+
Some(cx) => cx.spawn(future, name)
320+
}
323321
}
324322
}
325323

tokio/tests/task_panic.rs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,43 @@
33

44
use futures::future;
55
use std::error::Error;
6-
use tokio::{runtime::Builder, spawn, task};
6+
use tokio::runtime::Builder;
7+
use tokio::task::{self, block_in_place};
78

89
mod support {
910
pub mod panic;
1011
}
1112
use support::panic::test_panic;
1213

14+
#[test]
15+
fn block_in_place_panic_caller() -> Result<(), Box<dyn Error>> {
16+
let panic_location_file = test_panic(|| {
17+
let rt = Builder::new_current_thread().enable_all().build().unwrap();
18+
rt.block_on(async {
19+
block_in_place(|| {});
20+
});
21+
});
22+
23+
// The panic location should be in this file
24+
assert_eq!(&panic_location_file.unwrap(), file!());
25+
26+
Ok(())
27+
}
28+
29+
#[test]
30+
fn local_set_spawn_local_panic_caller() -> Result<(), Box<dyn Error>> {
31+
let panic_location_file = test_panic(|| {
32+
let _local = task::LocalSet::new();
33+
34+
let _ = task::spawn_local(async {});
35+
});
36+
37+
// The panic location should be in this file
38+
assert_eq!(&panic_location_file.unwrap(), file!());
39+
40+
Ok(())
41+
}
42+
1343
#[test]
1444
fn local_set_block_on_panic_caller() -> Result<(), Box<dyn Error>> {
1545
let panic_location_file = test_panic(|| {
@@ -30,7 +60,7 @@ fn local_set_block_on_panic_caller() -> Result<(), Box<dyn Error>> {
3060
#[test]
3161
fn spawn_panic_caller() -> Result<(), Box<dyn Error>> {
3262
let panic_location_file = test_panic(|| {
33-
spawn(future::pending::<()>());
63+
tokio::spawn(future::pending::<()>());
3464
});
3565

3666
// The panic location should be in this file

0 commit comments

Comments
 (0)