Skip to content

Commit 4e6456f

Browse files
committed
util: add track_caller to public APIs (tokio-rs#4413)
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 one place, an assert was added where the described behavior appeared not to be implemented. The documentation for `DelayQueue::reserve` states that the function will panic if the new capacity exceeds the maximum number of entries the queue can contain. However, the function didn't panic until a higher number caused by an allocation failure. This is inconsistent with `DelayQueue::insert_at` which will panic if the number of entries were to go over MAX_ENTRIES. Tests are included to cover each potentially panicking function.
1 parent 0196c31 commit 4e6456f

File tree

5 files changed

+204
-0
lines changed

5 files changed

+204
-0
lines changed

tokio-util/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ tokio-stream = { version = "0.1", path = "../tokio-stream" }
5555
async-stream = "0.3.0"
5656
futures = "0.3.0"
5757
futures-test = "0.3.5"
58+
parking_lot = "0.12.0"
5859

5960
[package.metadata.docs.rs]
6061
all-features = true

tokio-util/src/sync/mpsc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ impl<T: Send + 'static> PollSender<T> {
136136
///
137137
/// If `poll_reserve` was not successfully called prior to calling `send_item`, then this method
138138
/// will panic.
139+
#[track_caller]
139140
pub fn send_item(&mut self, value: T) -> Result<(), PollSendError<T>> {
140141
let (result, next_state) = match self.take_state() {
141142
State::Idle(_) | State::Acquiring => {

tokio-util/src/time/delay_queue.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,7 @@ impl<T> DelayQueue<T> {
531531
/// [`reset`]: method@Self::reset
532532
/// [`Key`]: struct@Key
533533
/// [type]: #
534+
#[track_caller]
534535
pub fn insert_at(&mut self, value: T, when: Instant) -> Key {
535536
assert!(self.slab.len() < MAX_ENTRIES, "max entries exceeded");
536537

@@ -649,10 +650,12 @@ impl<T> DelayQueue<T> {
649650
/// [`reset`]: method@Self::reset
650651
/// [`Key`]: struct@Key
651652
/// [type]: #
653+
#[track_caller]
652654
pub fn insert(&mut self, value: T, timeout: Duration) -> Key {
653655
self.insert_at(value, Instant::now() + timeout)
654656
}
655657

658+
#[track_caller]
656659
fn insert_idx(&mut self, when: u64, key: Key) {
657660
use self::wheel::{InsertError, Stack};
658661

@@ -674,6 +677,7 @@ impl<T> DelayQueue<T> {
674677
/// # Panics
675678
///
676679
/// Panics if the key is not contained in the expired queue or the wheel.
680+
#[track_caller]
677681
fn remove_key(&mut self, key: &Key) {
678682
use crate::time::wheel::Stack;
679683

@@ -713,6 +717,7 @@ impl<T> DelayQueue<T> {
713717
/// assert_eq!(*item.get_ref(), "foo");
714718
/// # }
715719
/// ```
720+
#[track_caller]
716721
pub fn remove(&mut self, key: &Key) -> Expired<T> {
717722
let prev_deadline = self.next_deadline();
718723

@@ -769,6 +774,7 @@ impl<T> DelayQueue<T> {
769774
/// // "foo" is now scheduled to be returned in 10 seconds
770775
/// # }
771776
/// ```
777+
#[track_caller]
772778
pub fn reset_at(&mut self, key: &Key, when: Instant) {
773779
self.remove_key(key);
774780

@@ -873,6 +879,7 @@ impl<T> DelayQueue<T> {
873879
/// // "foo"is now scheduled to be returned in 10 seconds
874880
/// # }
875881
/// ```
882+
#[track_caller]
876883
pub fn reset(&mut self, key: &Key, timeout: Duration) {
877884
self.reset_at(key, Instant::now() + timeout);
878885
}
@@ -978,7 +985,12 @@ impl<T> DelayQueue<T> {
978985
/// assert!(delay_queue.capacity() >= 11);
979986
/// # }
980987
/// ```
988+
#[track_caller]
981989
pub fn reserve(&mut self, additional: usize) {
990+
assert!(
991+
self.slab.capacity() + additional <= MAX_ENTRIES,
992+
"max queue capacity exceeded"
993+
);
982994
self.slab.reserve(additional);
983995
}
984996

@@ -1117,6 +1129,7 @@ impl<T> wheel::Stack for Stack<T> {
11171129
}
11181130
}
11191131

1132+
#[track_caller]
11201133
fn remove(&mut self, item: &Self::Borrowed, store: &mut Self::Store) {
11211134
let key = *item;
11221135
assert!(store.contains(item));

tokio-util/src/time/wheel/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ where
118118
}
119119

120120
/// Remove `item` from the timing wheel.
121+
#[track_caller]
121122
pub(crate) fn remove(&mut self, item: &T::Borrowed, store: &mut T::Store) {
122123
let when = T::when(item, store);
123124

tokio-util/tests/panic.rs

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
#![warn(rust_2018_idioms)]
2+
#![cfg(feature = "full")]
3+
4+
use parking_lot::{const_mutex, Mutex};
5+
use std::error::Error;
6+
use std::panic;
7+
use std::sync::Arc;
8+
use tokio::runtime::Runtime;
9+
use tokio::sync::mpsc::channel;
10+
use tokio::time::{Duration, Instant};
11+
use tokio_test::task;
12+
use tokio_util::sync::PollSender;
13+
use tokio_util::time::DelayQueue;
14+
15+
fn test_panic<Func: FnOnce() + panic::UnwindSafe>(func: Func) -> Option<String> {
16+
static PANIC_MUTEX: Mutex<()> = const_mutex(());
17+
18+
{
19+
let _guard = PANIC_MUTEX.lock();
20+
let panic_file: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));
21+
22+
let prev_hook = panic::take_hook();
23+
{
24+
let panic_file = panic_file.clone();
25+
panic::set_hook(Box::new(move |panic_info| {
26+
let panic_location = panic_info.location().unwrap();
27+
panic_file
28+
.lock()
29+
.clone_from(&Some(panic_location.file().to_string()));
30+
}));
31+
}
32+
33+
let result = panic::catch_unwind(func);
34+
// Return to the previously set panic hook (maybe default) so that we get nice error
35+
// messages in the tests.
36+
panic::set_hook(prev_hook);
37+
38+
if result.is_err() {
39+
panic_file.lock().clone()
40+
} else {
41+
None
42+
}
43+
}
44+
}
45+
46+
#[test]
47+
fn test_poll_sender_send_item_panic_caller() -> Result<(), Box<dyn Error>> {
48+
let panic_location_file = test_panic(|| {
49+
let (send, _) = channel::<u32>(3);
50+
let mut send = PollSender::new(send);
51+
52+
let _ = send.send_item(42);
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 test_delay_queue_insert_at_panic_caller() -> Result<(), Box<dyn Error>> {
63+
let panic_location_file = test_panic(|| {
64+
let rt = basic();
65+
rt.block_on(async {
66+
let mut queue = task::spawn(DelayQueue::with_capacity(3));
67+
68+
let _k = queue.insert_at(
69+
"1",
70+
// 36,500 days in the future.
71+
Instant::now() + Duration::from_secs(31_536_000_u64 * 100),
72+
);
73+
});
74+
});
75+
76+
// The panic location should be in this file
77+
assert_eq!(&panic_location_file.unwrap(), file!());
78+
79+
Ok(())
80+
}
81+
82+
#[test]
83+
fn test_delay_queue_insert_panic_caller() -> Result<(), Box<dyn Error>> {
84+
let panic_location_file = test_panic(|| {
85+
let rt = basic();
86+
rt.block_on(async {
87+
let mut queue = task::spawn(DelayQueue::with_capacity(3));
88+
89+
let _k = queue.insert(
90+
"1",
91+
// 36,500 days
92+
Duration::from_secs(31_536_000_u64 * 100),
93+
);
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 test_delay_queue_remove_panic_caller() -> Result<(), Box<dyn Error>> {
105+
let panic_location_file = test_panic(|| {
106+
let rt = basic();
107+
rt.block_on(async {
108+
let mut queue = task::spawn(DelayQueue::with_capacity(3));
109+
110+
let key = queue.insert_at("1", Instant::now());
111+
queue.remove(&key);
112+
queue.remove(&key);
113+
});
114+
});
115+
116+
// The panic location should be in this file
117+
assert_eq!(&panic_location_file.unwrap(), file!());
118+
119+
Ok(())
120+
}
121+
122+
#[test]
123+
fn test_delay_queue_reset_at_panic_caller() -> Result<(), Box<dyn Error>> {
124+
let panic_location_file = test_panic(|| {
125+
let rt = basic();
126+
rt.block_on(async {
127+
let mut queue = task::spawn(DelayQueue::with_capacity(3));
128+
129+
let key = queue.insert_at("1", Instant::now());
130+
queue.reset_at(
131+
&key,
132+
// 36,500 days in the future.
133+
Instant::now() + Duration::from_secs(31_536_000_u64 * 100),
134+
);
135+
});
136+
});
137+
138+
// The panic location should be in this file
139+
assert_eq!(&panic_location_file.unwrap(), file!());
140+
141+
Ok(())
142+
}
143+
144+
#[test]
145+
fn test_delay_queue_reset_panic_caller() -> Result<(), Box<dyn Error>> {
146+
let panic_location_file = test_panic(|| {
147+
let rt = basic();
148+
rt.block_on(async {
149+
let mut queue = task::spawn(DelayQueue::with_capacity(3));
150+
151+
let key = queue.insert_at("1", Instant::now());
152+
queue.reset(
153+
&key,
154+
// 36,500 days
155+
Duration::from_secs(31_536_000_u64 * 100),
156+
);
157+
});
158+
});
159+
160+
// The panic location should be in this file
161+
assert_eq!(&panic_location_file.unwrap(), file!());
162+
163+
Ok(())
164+
}
165+
166+
#[test]
167+
fn test_delay_queue_reserve_panic_caller() -> Result<(), Box<dyn Error>> {
168+
let panic_location_file = test_panic(|| {
169+
let rt = basic();
170+
rt.block_on(async {
171+
let mut queue = task::spawn(DelayQueue::<u32>::with_capacity(3));
172+
173+
queue.reserve((1 << 30) as usize);
174+
});
175+
});
176+
177+
// The panic location should be in this file
178+
assert_eq!(&panic_location_file.unwrap(), file!());
179+
180+
Ok(())
181+
}
182+
183+
fn basic() -> Runtime {
184+
tokio::runtime::Builder::new_current_thread()
185+
.enable_all()
186+
.build()
187+
.unwrap()
188+
}

0 commit comments

Comments
 (0)