io: add track_caller to public APIs (#4793)

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: #4413

Author: hds

tokio/src/io/async_fd.rs

@@ -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, or if the `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, or if the `rt`
+    /// feature flag is not enabled.
+    #[inline]
+    #[track_caller]
     pub fn with_interest(inner: T, interest: Interest) -> io::Result<Self>
     where
         T: AsRawFd,

tokio/src/io/driver/mod.rs

@@ -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.")
         }

tokio/src/io/read_buf.rs

@@ -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");
 
@@ -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);
@@ -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,
@@ -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(),

tokio/src/io/split.rs

@@ -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);

tokio/tests/io_panic.rs

@@ -0,0 +1,176 @@
+#![warn(rust_2018_idioms)]
+#![cfg(feature = "full")]
+
+use std::task::{Context, Poll};
+use std::{error::Error, pin::Pin};
+use tokio::io::{self, split, AsyncRead, AsyncWrite, ReadBuf};
+
+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(()))
+    }
+}
+
+#[cfg(unix)]
+mod unix {
+    use std::os::unix::prelude::{AsRawFd, RawFd};
+
+    pub 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>> {
+    use tokio::io::unix::AsyncFd;
+    use tokio::runtime::Builder;
+
+    let panic_location_file = test_panic(|| {
+        // Runtime without `enable_io` so it has no IO driver set.
+        let rt = Builder::new_current_thread().build().unwrap();
+        rt.block_on(async {
+            let fd = unix::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::unix::AsyncFd;
+    use tokio::io::Interest;
+    use tokio::runtime::Builder;
+
+    let panic_location_file = test_panic(|| {
+        // Runtime without `enable_io` so it has no IO driver set.
+        let rt = Builder::new_current_thread().build().unwrap();
+        rt.block_on(async {
+            let fd = unix::MockFd;
+
+            let _ = AsyncFd::with_interest(fd, Interest::READABLE);
+        });
+    });
+
+    // The panic location should be in this file
+    assert_eq!(&panic_location_file.unwrap(), file!());
+
+    Ok(())
+}

tokio/tests/rt_panic.rs

@@ -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>> {

tokio/tests/support/panic.rs

@@ -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
+        }
+    }
+}

tokio/tests/time_panic.rs

@@ -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>> {