Skip to content

Commit 4daeea8

Browse files
authored
sync: add track_caller to public APIs (#4808)
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 sync 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: * `sync::watch::Sender::send_modify` * `sync::watch::Sender::send_if_modified` Additionally, in the above functions it is a panic inside the supplied closure which causes the function to panic, and so showing the location of the panic itself is desirable. The following functions are async: * `sync::mpsc::bounded::Sender::send_timeout` Tests are included to cover each potentially panicking function. Refs: #4413
1 parent 18efef7 commit 4daeea8

File tree

9 files changed

+177
-0
lines changed

9 files changed

+177
-0
lines changed

tokio/src/future/block_on.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
use std::future::Future;
22

33
cfg_rt! {
4+
#[track_caller]
45
pub(crate) fn block_on<F: Future>(f: F) -> F::Output {
56
let mut e = crate::runtime::enter::enter(false);
67
e.block_on(f).unwrap()
78
}
89
}
910

1011
cfg_not_rt! {
12+
#[track_caller]
1113
pub(crate) fn block_on<F: Future>(f: F) -> F::Output {
1214
let mut park = crate::park::thread::CachedParkThread::new();
1315
park.block_on(f).unwrap()

tokio/src/runtime/enter.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ cfg_rt! {
3131

3232
/// Marks the current thread as being within the dynamic extent of an
3333
/// executor.
34+
#[track_caller]
3435
pub(crate) fn enter(allow_blocking: bool) -> Enter {
3536
if let Some(enter) = try_enter(allow_blocking) {
3637
return enter;

tokio/src/sync/broadcast.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,7 @@ const MAX_RECEIVERS: usize = usize::MAX >> 2;
430430
///
431431
/// This will panic if `capacity` is equal to `0` or larger
432432
/// than `usize::MAX / 2`.
433+
#[track_caller]
433434
pub fn channel<T: Clone>(mut capacity: usize) -> (Sender<T>, Receiver<T>) {
434435
assert!(capacity > 0, "capacity is empty");
435436
assert!(capacity <= usize::MAX >> 1, "requested capacity too large");

tokio/src/sync/mpsc/bounded.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ pub struct Receiver<T> {
105105
/// }
106106
/// }
107107
/// ```
108+
#[track_caller]
108109
pub fn channel<T>(buffer: usize) -> (Sender<T>, Receiver<T>) {
109110
assert!(buffer > 0, "mpsc bounded channel requires buffer > 0");
110111
let semaphore = (semaphore::Semaphore::new(buffer), buffer);
@@ -281,6 +282,7 @@ impl<T> Receiver<T> {
281282
/// sync_code.join().unwrap()
282283
/// }
283284
/// ```
285+
#[track_caller]
284286
#[cfg(feature = "sync")]
285287
pub fn blocking_recv(&mut self) -> Option<T> {
286288
crate::future::block_on(self.recv())
@@ -650,6 +652,7 @@ impl<T> Sender<T> {
650652
/// sync_code.join().unwrap()
651653
/// }
652654
/// ```
655+
#[track_caller]
653656
#[cfg(feature = "sync")]
654657
pub fn blocking_send(&self, value: T) -> Result<(), SendError<T>> {
655658
crate::future::block_on(self.send(value))

tokio/src/sync/mpsc/unbounded.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ impl<T> UnboundedReceiver<T> {
206206
/// sync_code.join().unwrap();
207207
/// }
208208
/// ```
209+
#[track_caller]
209210
#[cfg(feature = "sync")]
210211
pub fn blocking_recv(&mut self) -> Option<T> {
211212
crate::future::block_on(self.recv())

tokio/src/sync/mutex.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ impl<T: ?Sized> Mutex<T> {
411411
/// }
412412
///
413413
/// ```
414+
#[track_caller]
414415
#[cfg(feature = "sync")]
415416
pub fn blocking_lock(&self) -> MutexGuard<'_, T> {
416417
crate::future::block_on(self.lock())

tokio/src/sync/oneshot.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,7 @@ impl<T> Receiver<T> {
10521052
/// sync_code.join().unwrap();
10531053
/// }
10541054
/// ```
1055+
#[track_caller]
10551056
#[cfg(feature = "sync")]
10561057
pub fn blocking_recv(self) -> Result<T, RecvError> {
10571058
crate::future::block_on(self)

tokio/src/sync/rwlock.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ impl<T: ?Sized> RwLock<T> {
506506
/// assert!(rwlock.try_write().is_ok());
507507
/// }
508508
/// ```
509+
#[track_caller]
509510
#[cfg(feature = "sync")]
510511
pub fn blocking_read(&self) -> RwLockReadGuard<'_, T> {
511512
crate::future::block_on(self.read())
@@ -840,6 +841,7 @@ impl<T: ?Sized> RwLock<T> {
840841
/// assert_eq!(*read_lock, 2);
841842
/// }
842843
/// ```
844+
#[track_caller]
843845
#[cfg(feature = "sync")]
844846
pub fn blocking_write(&self) -> RwLockWriteGuard<'_, T> {
845847
crate::future::block_on(self.write())

tokio/tests/sync_panic.rs

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
#![warn(rust_2018_idioms)]
2+
#![cfg(feature = "full")]
3+
4+
use std::error::Error;
5+
use tokio::{
6+
runtime::{Builder, Runtime},
7+
sync::{broadcast, mpsc, oneshot, Mutex, RwLock},
8+
};
9+
10+
mod support {
11+
pub mod panic;
12+
}
13+
use support::panic::test_panic;
14+
15+
#[test]
16+
fn broadcast_channel_panic_caller() -> Result<(), Box<dyn Error>> {
17+
let panic_location_file = test_panic(|| {
18+
let (_, _) = broadcast::channel::<u32>(0);
19+
});
20+
21+
// The panic location should be in this file
22+
assert_eq!(&panic_location_file.unwrap(), file!());
23+
24+
Ok(())
25+
}
26+
27+
#[test]
28+
fn mutex_blocking_lock_panic_caller() -> Result<(), Box<dyn Error>> {
29+
let panic_location_file = test_panic(|| {
30+
let rt = basic();
31+
rt.block_on(async {
32+
let mutex = Mutex::new(5_u32);
33+
mutex.blocking_lock();
34+
});
35+
});
36+
37+
// The panic location should be in this file
38+
assert_eq!(&panic_location_file.unwrap(), file!());
39+
40+
Ok(())
41+
}
42+
43+
#[test]
44+
fn oneshot_blocking_recv_panic_caller() -> Result<(), Box<dyn Error>> {
45+
let panic_location_file = test_panic(|| {
46+
let rt = basic();
47+
rt.block_on(async {
48+
let (_tx, rx) = oneshot::channel::<u8>();
49+
let _ = rx.blocking_recv();
50+
});
51+
});
52+
53+
// The panic location should be in this file
54+
assert_eq!(&panic_location_file.unwrap(), file!());
55+
56+
Ok(())
57+
}
58+
59+
#[test]
60+
fn rwlock_with_max_readers_panic_caller() -> Result<(), Box<dyn Error>> {
61+
let panic_location_file = test_panic(|| {
62+
let _ = RwLock::<u8>::with_max_readers(0, (u32::MAX >> 3) + 1);
63+
});
64+
65+
// The panic location should be in this file
66+
assert_eq!(&panic_location_file.unwrap(), file!());
67+
68+
Ok(())
69+
}
70+
71+
#[test]
72+
fn rwlock_blocking_read_panic_caller() -> Result<(), Box<dyn Error>> {
73+
let panic_location_file = test_panic(|| {
74+
let rt = basic();
75+
rt.block_on(async {
76+
let lock = RwLock::<u8>::new(0);
77+
let _ = lock.blocking_read();
78+
});
79+
});
80+
81+
// The panic location should be in this file
82+
assert_eq!(&panic_location_file.unwrap(), file!());
83+
84+
Ok(())
85+
}
86+
87+
#[test]
88+
fn rwlock_blocking_write_panic_caller() -> Result<(), Box<dyn Error>> {
89+
let panic_location_file = test_panic(|| {
90+
let rt = basic();
91+
rt.block_on(async {
92+
let lock = RwLock::<u8>::new(0);
93+
let _ = lock.blocking_write();
94+
});
95+
});
96+
97+
// The panic location should be in this file
98+
assert_eq!(&panic_location_file.unwrap(), file!());
99+
100+
Ok(())
101+
}
102+
103+
#[test]
104+
fn mpsc_bounded_channel_panic_caller() -> Result<(), Box<dyn Error>> {
105+
let panic_location_file = test_panic(|| {
106+
let (_, _) = mpsc::channel::<u8>(0);
107+
});
108+
109+
// The panic location should be in this file
110+
assert_eq!(&panic_location_file.unwrap(), file!());
111+
112+
Ok(())
113+
}
114+
115+
#[test]
116+
fn mpsc_bounded_receiver_blocking_recv_panic_caller() -> Result<(), Box<dyn Error>> {
117+
let panic_location_file = test_panic(|| {
118+
let rt = basic();
119+
let (_tx, mut rx) = mpsc::channel::<u8>(1);
120+
rt.block_on(async {
121+
let _ = rx.blocking_recv();
122+
});
123+
});
124+
125+
// The panic location should be in this file
126+
assert_eq!(&panic_location_file.unwrap(), file!());
127+
128+
Ok(())
129+
}
130+
131+
#[test]
132+
fn mpsc_bounded_sender_blocking_send_panic_caller() -> Result<(), Box<dyn Error>> {
133+
let panic_location_file = test_panic(|| {
134+
let rt = basic();
135+
let (tx, _rx) = mpsc::channel::<u8>(1);
136+
rt.block_on(async {
137+
let _ = tx.blocking_send(3);
138+
});
139+
});
140+
141+
// The panic location should be in this file
142+
assert_eq!(&panic_location_file.unwrap(), file!());
143+
144+
Ok(())
145+
}
146+
147+
#[test]
148+
fn mpsc_unbounded_receiver_blocking_recv_panic_caller() -> Result<(), Box<dyn Error>> {
149+
let panic_location_file = test_panic(|| {
150+
let rt = basic();
151+
let (_tx, mut rx) = mpsc::unbounded_channel::<u8>();
152+
rt.block_on(async {
153+
let _ = rx.blocking_recv();
154+
});
155+
});
156+
157+
// The panic location should be in this file
158+
assert_eq!(&panic_location_file.unwrap(), file!());
159+
160+
Ok(())
161+
}
162+
163+
fn basic() -> Runtime {
164+
Builder::new_current_thread().enable_all().build().unwrap()
165+
}

0 commit comments

Comments
 (0)