Skip to content

Commit

Permalink
io: add track_caller to public APIs
Browse files Browse the repository at this point in the history
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 io APIs
in the main tokio crate where the documentation describes how the
function may panic due to incorrect context or inputs.

Additionally, the documentation for `AsyncFd` was updated to indicate
that the functions `new` and `with_intent` can panic.

Tests are included to cover each potentially panicking function. The
logic to test the location of a panic (which is a little complex), has
been moved to a test support module.

Refs: tokio-rs#4413
  • Loading branch information
hds committed Jun 27, 2022
1 parent 3cc6168 commit 2ab60c2
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 66 deletions.
16 changes: 14 additions & 2 deletions tokio/src/io/async_fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,22 +167,34 @@ pub struct AsyncFdReadyMutGuard<'a, T: AsRawFd> {
const ALL_INTEREST: Interest = Interest::READABLE.add(Interest::WRITABLE);

impl<T: AsRawFd> AsyncFd<T> {
#[inline]
/// Creates an AsyncFd backed by (and taking ownership of) an object
/// implementing [`AsRawFd`]. The backing file descriptor is cached at the
/// time of creation.
///
/// This method must be called in the context of a tokio runtime.
///
/// # Panics
///
/// This function panics if there is no current reactor set and `rt` feature
/// flag is not enabled.
#[inline]
#[track_caller]
pub fn new(inner: T) -> io::Result<Self>
where
T: AsRawFd,
{
Self::with_interest(inner, ALL_INTEREST)
}

#[inline]
/// Creates new instance as `new` with additional ability to customize interest,
/// allowing to specify whether file descriptor will be polled for read, write or both.
///
/// # Panics
///
/// This function panics if there is no current reactor set and `rt` feature
/// flag is not enabled.
#[inline]
#[track_caller]
pub fn with_interest(inner: T, interest: Interest) -> io::Result<Self>
where
T: AsRawFd,
Expand Down
1 change: 1 addition & 0 deletions tokio/src/io/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ cfg_rt! {
///
/// This function panics if there is no current reactor set and `rt` feature
/// flag is not enabled.
#[track_caller]
pub(super) fn current() -> Self {
crate::runtime::context::io_handle().expect("A Tokio 1.x context was found, but IO is disabled. Call `enable_io` on the runtime builder to enable IO.")
}
Expand Down
4 changes: 4 additions & 0 deletions tokio/src/io/read_buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl<'a> ReadBuf<'a> {
///
/// Panics if `self.remaining()` is less than `n`.
#[inline]
#[track_caller]
pub fn initialize_unfilled_to(&mut self, n: usize) -> &mut [u8] {
assert!(self.remaining() >= n, "n overflows remaining");

Expand Down Expand Up @@ -195,6 +196,7 @@ impl<'a> ReadBuf<'a> {
///
/// Panics if the filled region of the buffer would become larger than the initialized region.
#[inline]
#[track_caller]
pub fn advance(&mut self, n: usize) {
let new = self.filled.checked_add(n).expect("filled overflow");
self.set_filled(new);
Expand All @@ -211,6 +213,7 @@ impl<'a> ReadBuf<'a> {
///
/// Panics if the filled region of the buffer would become larger than the initialized region.
#[inline]
#[track_caller]
pub fn set_filled(&mut self, n: usize) {
assert!(
n <= self.initialized,
Expand Down Expand Up @@ -241,6 +244,7 @@ impl<'a> ReadBuf<'a> {
///
/// Panics if `self.remaining()` is less than `buf.len()`.
#[inline]
#[track_caller]
pub fn put_slice(&mut self, buf: &[u8]) {
assert!(
self.remaining() >= buf.len(),
Expand Down
1 change: 1 addition & 0 deletions tokio/src/io/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl<T> ReadHalf<T> {
/// same `split` operation this method will panic.
/// This can be checked ahead of time by comparing the stream ID
/// of the two halves.
#[track_caller]
pub fn unsplit(self, wr: WriteHalf<T>) -> T {
if self.is_pair_of(&wr) {
drop(wr);
Expand Down
168 changes: 168 additions & 0 deletions tokio/tests/io_panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
#![warn(rust_2018_idioms)]
#![cfg(feature = "full")]

use std::os::unix::prelude::{AsRawFd, RawFd};
use std::task::{Context, Poll};
use std::{error::Error, pin::Pin};
use tokio::io::{self, split, unix::AsyncFd, AsyncRead, AsyncWrite, ReadBuf};
use tokio::runtime::Builder;

mod support {
pub mod panic;
}
use support::panic::test_panic;

struct RW;

impl AsyncRead for RW {
fn poll_read(
self: Pin<&mut Self>,
_cx: &mut Context<'_>,
buf: &mut ReadBuf<'_>,
) -> Poll<io::Result<()>> {
buf.put_slice(&[b'z']);
Poll::Ready(Ok(()))
}
}

impl AsyncWrite for RW {
fn poll_write(
self: Pin<&mut Self>,
_cx: &mut Context<'_>,
_buf: &[u8],
) -> Poll<Result<usize, io::Error>> {
Poll::Ready(Ok(1))
}

fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Result<(), io::Error>> {
Poll::Ready(Ok(()))
}

fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Result<(), io::Error>> {
Poll::Ready(Ok(()))
}
}

struct MockFd;

impl AsRawFd for MockFd {
fn as_raw_fd(&self) -> RawFd {
0
}
}

#[test]
fn read_buf_initialize_unfilled_to_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let mut buffer = Vec::<u8>::new();
let mut read_buf = ReadBuf::new(&mut buffer);

read_buf.initialize_unfilled_to(2);
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn read_buf_advance_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let mut buffer = Vec::<u8>::new();
let mut read_buf = ReadBuf::new(&mut buffer);

read_buf.advance(2);
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn read_buf_set_filled_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let mut buffer = Vec::<u8>::new();
let mut read_buf = ReadBuf::new(&mut buffer);

read_buf.set_filled(2);
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn read_buf_put_slice_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let mut buffer = Vec::<u8>::new();
let mut read_buf = ReadBuf::new(&mut buffer);

let new_slice = [0x40_u8, 0x41_u8];

read_buf.put_slice(&new_slice);
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
fn unsplit_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
let (r1, _w1) = split(RW);
let (_r2, w2) = split(RW);
r1.unsplit(w2);
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
#[cfg(unix)]
fn async_fd_new_panic_caller() -> Result<(), Box<dyn Error>> {
let panic_location_file = test_panic(|| {
// Runtime without `enable_io` so it has no current timer set.
let rt = Builder::new_current_thread().build().unwrap();
rt.block_on(async {
let fd = MockFd;

let _ = AsyncFd::new(fd);
});
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}

#[test]
#[cfg(unix)]
fn async_fd_with_interest_panic_caller() -> Result<(), Box<dyn Error>> {
use tokio::io::Interest;

let panic_location_file = test_panic(|| {
// Runtime without `enable_io` so it has no current timer set.
let rt = Builder::new_current_thread().build().unwrap();
rt.block_on(async {
let fd = MockFd;

let _ = AsyncFd::with_interest(fd, Interest::READABLE);
});
});

// The panic location should be in this file
assert_eq!(&panic_location_file.unwrap(), file!());

Ok(())
}
35 changes: 3 additions & 32 deletions tokio/tests/rt_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,13 @@
#![cfg(feature = "full")]

use futures::future;
use parking_lot::{const_mutex, Mutex};
use std::error::Error;
use std::panic;
use std::sync::Arc;
use tokio::runtime::{Builder, Handle, Runtime};

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
}
}
mod support {
pub mod panic;
}
use support::panic::test_panic;

#[test]
fn current_handle_panic_caller() -> Result<(), Box<dyn Error>> {
Expand Down
34 changes: 34 additions & 0 deletions tokio/tests/support/panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use parking_lot::{const_mutex, Mutex};
use std::panic;
use std::sync::Arc;

pub 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
}
}
}
35 changes: 3 additions & 32 deletions tokio/tests/time_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,15 @@
#![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
}
}
mod support {
pub mod panic;
}
use support::panic::test_panic;

#[test]
fn pause_panic_caller() -> Result<(), Box<dyn Error>> {
Expand Down

0 comments on commit 2ab60c2

Please sign in to comment.