From 3eb71f2ca30404a026f32359535c8c5a1b8362b8 Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Mon, 4 Jul 2022 12:54:39 +0200 Subject: [PATCH 1/7] net: add track_caller to public APIs 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 net APIs in the main tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. Not all panic cases can have #[track_caller] applied fully as the callstack passes through a closure which isn't yet supported by the annotation (e.g. net functions called from outside a tokio runtime). Additionally, the documentation was updated to indicate additional cases in which the public net functions may panic (the same as the io functions). Tests are included to cover each potentially panicking function. Refs: #4413 --- tokio/src/io/poll_evented.rs | 2 + tokio/src/net/tcp/listener.rs | 4 +- tokio/src/net/tcp/stream.rs | 4 +- tokio/src/net/udp.rs | 2 + tokio/src/net/unix/datagram/socket.rs | 4 +- tokio/src/net/unix/listener.rs | 8 +- tokio/src/net/unix/stream.rs | 4 +- tokio/src/runtime/context.rs | 1 + tokio/tests/net_panic.rs | 188 ++++++++++++++++++++++++++ 9 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 tokio/tests/net_panic.rs diff --git a/tokio/src/io/poll_evented.rs b/tokio/src/io/poll_evented.rs index ce4c1426acc..df071897d91 100644 --- a/tokio/src/io/poll_evented.rs +++ b/tokio/src/io/poll_evented.rs @@ -77,6 +77,7 @@ impl PollEvented { /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] #[cfg_attr(feature = "signal", allow(unused))] pub(crate) fn new(io: E) -> io::Result { PollEvented::new_with_interest(io, Interest::READABLE | Interest::WRITABLE) @@ -97,6 +98,7 @@ impl PollEvented { /// a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) /// function. + #[track_caller] #[cfg_attr(feature = "signal", allow(unused))] pub(crate) fn new_with_interest(io: E, interest: Interest) -> io::Result { Self::new_with_interest_and_handle(io, interest, Handle::current()) diff --git a/tokio/src/net/tcp/listener.rs b/tokio/src/net/tcp/listener.rs index 8aecb21aaa9..c5d3491054c 100644 --- a/tokio/src/net/tcp/listener.rs +++ b/tokio/src/net/tcp/listener.rs @@ -216,11 +216,13 @@ impl TcpListener { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] pub fn from_std(listener: net::TcpListener) -> io::Result { let io = mio::net::TcpListener::from_std(listener); let io = PollEvented::new(io)?; diff --git a/tokio/src/net/tcp/stream.rs b/tokio/src/net/tcp/stream.rs index 204d9ca256c..d9191309e92 100644 --- a/tokio/src/net/tcp/stream.rs +++ b/tokio/src/net/tcp/stream.rs @@ -181,11 +181,13 @@ impl TcpStream { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] pub fn from_std(stream: std::net::TcpStream) -> io::Result { let io = mio::net::TcpStream::from_std(stream); let io = PollEvented::new(io)?; diff --git a/tokio/src/net/udp.rs b/tokio/src/net/udp.rs index bd905e91a5f..218029176e0 100644 --- a/tokio/src/net/udp.rs +++ b/tokio/src/net/udp.rs @@ -170,6 +170,7 @@ impl UdpSocket { UdpSocket::new(sys) } + #[track_caller] fn new(socket: mio::net::UdpSocket) -> io::Result { let io = PollEvented::new(socket)?; Ok(UdpSocket { io }) @@ -210,6 +211,7 @@ impl UdpSocket { /// # Ok(()) /// # } /// ``` + #[track_caller] pub fn from_std(socket: net::UdpSocket) -> io::Result { let io = mio::net::UdpSocket::from_std(socket); UdpSocket::new(io) diff --git a/tokio/src/net/unix/datagram/socket.rs b/tokio/src/net/unix/datagram/socket.rs index def006c4761..275eb14325c 100644 --- a/tokio/src/net/unix/datagram/socket.rs +++ b/tokio/src/net/unix/datagram/socket.rs @@ -430,7 +430,8 @@ impl UnixDatagram { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a Tokio runtime, otherwise runtime can be set @@ -457,6 +458,7 @@ impl UnixDatagram { /// # Ok(()) /// # } /// ``` + #[track_caller] pub fn from_std(datagram: net::UnixDatagram) -> io::Result { let socket = mio::net::UnixDatagram::from_std(datagram); let io = PollEvented::new(socket)?; diff --git a/tokio/src/net/unix/listener.rs b/tokio/src/net/unix/listener.rs index 1785f8b0f73..35da60457a2 100644 --- a/tokio/src/net/unix/listener.rs +++ b/tokio/src/net/unix/listener.rs @@ -54,11 +54,13 @@ impl UnixListener { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] pub fn bind

(path: P) -> io::Result where P: AsRef, @@ -77,11 +79,13 @@ impl UnixListener { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] pub fn from_std(listener: net::UnixListener) -> io::Result { let listener = mio::net::UnixListener::from_std(listener); let io = PollEvented::new(listener)?; diff --git a/tokio/src/net/unix/stream.rs b/tokio/src/net/unix/stream.rs index fe2d825bf98..a24836f7dee 100644 --- a/tokio/src/net/unix/stream.rs +++ b/tokio/src/net/unix/stream.rs @@ -699,11 +699,13 @@ impl UnixStream { /// /// # Panics /// - /// This function panics if thread-local runtime is not set. + /// This function panics if there is no current reactor set, if the `rt` + /// feature flag is not enabled, or if thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set /// explicitly with [`Runtime::enter`](crate::runtime::Runtime::enter) function. + #[track_caller] pub fn from_std(stream: net::UnixStream) -> io::Result { let stream = mio::net::UnixStream::from_std(stream); let io = PollEvented::new(stream)?; diff --git a/tokio/src/runtime/context.rs b/tokio/src/runtime/context.rs index 36fa28db356..4215124fc83 100644 --- a/tokio/src/runtime/context.rs +++ b/tokio/src/runtime/context.rs @@ -24,6 +24,7 @@ pub(crate) fn current() -> Handle { } cfg_io_driver! { + #[track_caller] pub(crate) fn io_handle() -> crate::runtime::driver::IoHandle { match CONTEXT.try_with(|ctx| { let ctx = ctx.borrow(); diff --git a/tokio/tests/net_panic.rs b/tokio/tests/net_panic.rs new file mode 100644 index 00000000000..835d31fa51d --- /dev/null +++ b/tokio/tests/net_panic.rs @@ -0,0 +1,188 @@ +#![warn(rust_2018_idioms)] +#![cfg(feature = "full")] + +use std::error::Error; +use tokio::net::{TcpListener, TcpStream}; +use tokio::runtime::{Builder, Runtime}; + +mod support { + pub mod panic; +} +use support::panic::test_panic; + +#[test] +fn udp_socket_from_std_panic_caller() -> Result<(), Box> { + use std::net::SocketAddr; + use tokio::net::UdpSocket; + + let addr = "127.0.0.1:8080".parse::().unwrap(); + let std_sock = std::net::UdpSocket::bind(addr).unwrap(); + std_sock.set_nonblocking(true).unwrap(); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _sock = UdpSocket::from_std(std_sock); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn tcp_listener_from_std_panic_caller() -> Result<(), Box> { + let std_listener = std::net::TcpListener::bind("127.0.0.1:8080").unwrap(); + std_listener.set_nonblocking(true).unwrap(); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = TcpListener::from_std(std_listener); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn tcp_stream_from_std_panic_caller() -> Result<(), Box> { + let std_listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + + let std_stream = std::net::TcpStream::connect(std_listener.local_addr().unwrap()).unwrap(); + std_stream.set_nonblocking(true).unwrap(); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = TcpStream::from_std(std_stream); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn unix_listener_bind_panic_caller() -> Result<(), Box> { + use tokio::net::UnixListener; + + let dir = tempfile::tempdir().unwrap(); + let sock_path = dir.path().join("socket"); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = UnixListener::bind(&sock_path); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn unix_listener_from_std_panic_caller() -> Result<(), Box> { + use tokio::net::UnixListener; + + let dir = tempfile::tempdir().unwrap(); + let sock_path = dir.path().join("socket"); + let std_listener = std::os::unix::net::UnixListener::bind(&sock_path).unwrap(); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = UnixListener::from_std(std_listener); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn unix_stream_from_std_panic_caller() -> Result<(), Box> { + use tokio::net::UnixStream; + + let dir = tempfile::tempdir().unwrap(); + let sock_path = dir.path().join("socket"); + let _std_listener = std::os::unix::net::UnixListener::bind(&sock_path).unwrap(); + let std_stream = std::os::unix::net::UnixStream::connect(&sock_path).unwrap(); + + let panic_location_file = test_panic(|| { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = UnixStream::from_std(std_stream); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn unix_datagram_from_std_panic_caller() -> Result<(), Box> { + use std::os::unix::net::UnixDatagram as StdUDS; + use tokio::net::UnixDatagram; + + let dir = tempfile::tempdir().unwrap(); + let sock_path = dir.path().join("socket"); + + // Bind the socket to a filesystem path + // /let socket_path = tmp.path().join("socket"); + let std_socket = StdUDS::bind(&sock_path).unwrap(); + std_socket.set_nonblocking(true).unwrap(); + + let panic_location_file = test_panic(move || { + let rt = runtime_without_io(); + rt.block_on(async { + let _ = UnixDatagram::from_std(std_socket); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +#[cfg(windows)] +fn named_pipe_count_panic_caller() -> Result<(), Box> { + use tokio::net::windows::named_pipe::ServerOptions; + + let panic_location_file = test_panic(move || { + let rt = runtime_without_io(); + rt.block_on(async { + let mut options = ServerOptions::new(); + options.max_instances(255); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +// Runtime without `enable_io` so it has no IO driver set. +fn runtime_without_io() -> Runtime { + Builder::new_current_thread().build().unwrap() +} From 62a74559f2205a7b6219fddf223477c7c23d7252 Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Tue, 5 Jul 2022 00:55:15 +0200 Subject: [PATCH 2/7] Added track_caller to max_instances Without a windows machine to test on, I had to add the test before adding track_caller to ensure that it failed first. This commit adds the change that should make the test pass (and also fixes the test name). --- tokio/src/net/windows/named_pipe.rs | 1 + tokio/tests/net_panic.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tokio/src/net/windows/named_pipe.rs b/tokio/src/net/windows/named_pipe.rs index 695b8eb3d39..343012f1aa9 100644 --- a/tokio/src/net/windows/named_pipe.rs +++ b/tokio/src/net/windows/named_pipe.rs @@ -2020,6 +2020,7 @@ impl ServerOptions { /// let builder = ServerOptions::new().max_instances(255); /// # Ok(()) } /// ``` + #[track_caller] pub fn max_instances(&mut self, instances: usize) -> &mut Self { assert!(instances < 255, "cannot specify more than 254 instances"); self.max_instances = instances as DWORD; diff --git a/tokio/tests/net_panic.rs b/tokio/tests/net_panic.rs index 835d31fa51d..a81ef479b1c 100644 --- a/tokio/tests/net_panic.rs +++ b/tokio/tests/net_panic.rs @@ -165,7 +165,7 @@ fn unix_datagram_from_std_panic_caller() -> Result<(), Box> { #[test] #[cfg(windows)] -fn named_pipe_count_panic_caller() -> Result<(), Box> { +fn server_options_max_instances_panic_caller() -> Result<(), Box> { use tokio::net::windows::named_pipe::ServerOptions; let panic_location_file = test_panic(move || { From 0918514fdf46ae59687d6b0df13ba538d8a0c11b Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Tue, 5 Jul 2022 17:50:05 +0200 Subject: [PATCH 3/7] sync: add track_caller to public APIs 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 --- tokio/src/future/block_on.rs | 2 + tokio/src/runtime/enter.rs | 1 + tokio/src/sync/broadcast.rs | 1 + tokio/src/sync/mpsc/bounded.rs | 2 + tokio/src/sync/mutex.rs | 1 + tokio/src/sync/oneshot.rs | 1 + tokio/src/sync/rwlock.rs | 2 + tokio/tests/sync_panic.rs | 157 +++++++++++++++++++++++++++++++++ 8 files changed, 167 insertions(+) create mode 100644 tokio/tests/sync_panic.rs diff --git a/tokio/src/future/block_on.rs b/tokio/src/future/block_on.rs index 91f9cc00550..8e1e6957eee 100644 --- a/tokio/src/future/block_on.rs +++ b/tokio/src/future/block_on.rs @@ -1,6 +1,7 @@ use std::future::Future; cfg_rt! { + #[track_caller] pub(crate) fn block_on(f: F) -> F::Output { let mut e = crate::runtime::enter::enter(false); e.block_on(f).unwrap() @@ -8,6 +9,7 @@ cfg_rt! { } cfg_not_rt! { + #[track_caller] pub(crate) fn block_on(f: F) -> F::Output { let mut park = crate::park::thread::CachedParkThread::new(); park.block_on(f).unwrap() diff --git a/tokio/src/runtime/enter.rs b/tokio/src/runtime/enter.rs index 7d2c7773832..6e4e37d70ff 100644 --- a/tokio/src/runtime/enter.rs +++ b/tokio/src/runtime/enter.rs @@ -31,6 +31,7 @@ cfg_rt! { /// Marks the current thread as being within the dynamic extent of an /// executor. + #[track_caller] pub(crate) fn enter(allow_blocking: bool) -> Enter { if let Some(enter) = try_enter(allow_blocking) { return enter; diff --git a/tokio/src/sync/broadcast.rs b/tokio/src/sync/broadcast.rs index c796d129920..fa6f46b0a4c 100644 --- a/tokio/src/sync/broadcast.rs +++ b/tokio/src/sync/broadcast.rs @@ -430,6 +430,7 @@ const MAX_RECEIVERS: usize = usize::MAX >> 2; /// /// This will panic if `capacity` is equal to `0` or larger /// than `usize::MAX / 2`. +#[track_caller] pub fn channel(mut capacity: usize) -> (Sender, Receiver) { assert!(capacity > 0, "capacity is empty"); assert!(capacity <= usize::MAX >> 1, "requested capacity too large"); diff --git a/tokio/src/sync/mpsc/bounded.rs b/tokio/src/sync/mpsc/bounded.rs index ddded8ebb31..5ec2ecd9b97 100644 --- a/tokio/src/sync/mpsc/bounded.rs +++ b/tokio/src/sync/mpsc/bounded.rs @@ -105,6 +105,7 @@ pub struct Receiver { /// } /// } /// ``` +#[track_caller] pub fn channel(buffer: usize) -> (Sender, Receiver) { assert!(buffer > 0, "mpsc bounded channel requires buffer > 0"); let semaphore = (semaphore::Semaphore::new(buffer), buffer); @@ -281,6 +282,7 @@ impl Receiver { /// sync_code.join().unwrap() /// } /// ``` + #[track_caller] #[cfg(feature = "sync")] pub fn blocking_recv(&mut self) -> Option { crate::future::block_on(self.recv()) diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs index b8d5ba74e75..4c9899f940b 100644 --- a/tokio/src/sync/mutex.rs +++ b/tokio/src/sync/mutex.rs @@ -411,6 +411,7 @@ impl Mutex { /// } /// /// ``` + #[track_caller] #[cfg(feature = "sync")] pub fn blocking_lock(&self) -> MutexGuard<'_, T> { crate::future::block_on(self.lock()) diff --git a/tokio/src/sync/oneshot.rs b/tokio/src/sync/oneshot.rs index 8ee0ef702d4..07b39d077b7 100644 --- a/tokio/src/sync/oneshot.rs +++ b/tokio/src/sync/oneshot.rs @@ -1052,6 +1052,7 @@ impl Receiver { /// sync_code.join().unwrap(); /// } /// ``` + #[track_caller] #[cfg(feature = "sync")] pub fn blocking_recv(self) -> Result { crate::future::block_on(self) diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs index b856cfc8567..0492e4e9d39 100644 --- a/tokio/src/sync/rwlock.rs +++ b/tokio/src/sync/rwlock.rs @@ -506,6 +506,7 @@ impl RwLock { /// assert!(rwlock.try_write().is_ok()); /// } /// ``` + #[track_caller] #[cfg(feature = "sync")] pub fn blocking_read(&self) -> RwLockReadGuard<'_, T> { crate::future::block_on(self.read()) @@ -840,6 +841,7 @@ impl RwLock { /// assert_eq!(*read_lock, 2); /// } /// ``` + #[track_caller] #[cfg(feature = "sync")] pub fn blocking_write(&self) -> RwLockWriteGuard<'_, T> { crate::future::block_on(self.write()) diff --git a/tokio/tests/sync_panic.rs b/tokio/tests/sync_panic.rs new file mode 100644 index 00000000000..5dab15a0462 --- /dev/null +++ b/tokio/tests/sync_panic.rs @@ -0,0 +1,157 @@ +#![warn(rust_2018_idioms)] +#![cfg(feature = "full")] + +use std::error::Error; + +use tokio::{ + runtime::{Builder, Runtime}, + sync::{broadcast, mpsc, oneshot, Mutex, RwLock}, + time::Duration, +}; + +mod support { + pub mod panic; +} +use support::panic::test_panic; + +#[test] +fn broadcast_channel_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let (_, _) = broadcast::channel::(0); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn mutex_blocking_lock_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let rt = basic(); + rt.block_on(async { + let mutex = Mutex::new(5_u32); + mutex.blocking_lock(); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn oneshot_blocking_recv_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let rt = basic(); + rt.block_on(async { + let (_tx, rx) = oneshot::channel::(); + let _ = rx.blocking_recv(); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn rwlock_with_max_readers_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let _ = RwLock::::with_max_readers(0, (u32::MAX >> 3) + 1); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn rwlock_blocking_read_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let rt = basic(); + rt.block_on(async { + let lock = RwLock::::new(0); + let _ = lock.blocking_read(); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn rwlock_blocking_write_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let rt = basic(); + rt.block_on(async { + let lock = RwLock::::new(0); + let _ = lock.blocking_write(); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn mpsc_bounded_channel_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let (_, _) = mpsc::channel::(0); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn mpsc_bounded_receiver_blocking_recv_panic_caller() -> Result<(), Box> { + let panic_location_file = test_panic(|| { + let rt = basic(); + let (_tx, mut rx) = mpsc::channel::(1); + rt.block_on(async { + let _ = rx.blocking_recv(); + }); + }); + + // The panic location should be in this file + assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} + +#[test] +fn mpsc_bounded_sender_send_timeout_panic_caller() -> Result<(), Box> { + let rt = Builder::new_current_thread().build().unwrap(); + let (tx, _rx) = mpsc::channel::(1); + rt.block_on(async { + let _ = tx.send_timeout(3, Duration::from_millis(2)).await; + }); + + // let panic_location_file = test_panic(|| { + // // Runtime without time + // let rt = Builder::new_current_thread().build().unwrap(); + // let (tx, _rx) = mpsc::channel::(1); + // rt.block_on(async { + // let _ = tx.send_timeout(3, Duration::from_millis(2)); + // }); + // }); + + // // The panic location should be in this file + // assert_eq!(&panic_location_file.unwrap(), file!()); + + Ok(()) +} +fn basic() -> Runtime { + Builder::new_current_thread().enable_all().build().unwrap() +} From dedc60e1e26ac48bd7b780f82d80f680ee25d89d Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Thu, 7 Jul 2022 01:34:41 +0200 Subject: [PATCH 4/7] Modified changes to some panic descriptions Modified the description of situations in which some functions can panic to align with reviewer suggestions (at least that's the intent). --- tokio/src/net/tcp/listener.rs | 4 ++-- tokio/src/net/tcp/stream.rs | 4 ++-- tokio/src/net/unix/datagram/socket.rs | 4 ++-- tokio/src/net/unix/listener.rs | 8 ++++---- tokio/src/net/unix/stream.rs | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tokio/src/net/tcp/listener.rs b/tokio/src/net/tcp/listener.rs index c5d3491054c..ea893f41079 100644 --- a/tokio/src/net/tcp/listener.rs +++ b/tokio/src/net/tcp/listener.rs @@ -216,8 +216,8 @@ impl TcpListener { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/tcp/stream.rs b/tokio/src/net/tcp/stream.rs index d9191309e92..1337d07185c 100644 --- a/tokio/src/net/tcp/stream.rs +++ b/tokio/src/net/tcp/stream.rs @@ -181,8 +181,8 @@ impl TcpStream { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/datagram/socket.rs b/tokio/src/net/unix/datagram/socket.rs index 275eb14325c..963b20f3fb2 100644 --- a/tokio/src/net/unix/datagram/socket.rs +++ b/tokio/src/net/unix/datagram/socket.rs @@ -430,8 +430,8 @@ impl UnixDatagram { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a Tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/listener.rs b/tokio/src/net/unix/listener.rs index 35da60457a2..f0106eefe4f 100644 --- a/tokio/src/net/unix/listener.rs +++ b/tokio/src/net/unix/listener.rs @@ -54,8 +54,8 @@ impl UnixListener { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set @@ -79,8 +79,8 @@ impl UnixListener { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/stream.rs b/tokio/src/net/unix/stream.rs index a24836f7dee..f11025e76d3 100644 --- a/tokio/src/net/unix/stream.rs +++ b/tokio/src/net/unix/stream.rs @@ -699,8 +699,8 @@ impl UnixStream { /// /// # Panics /// - /// This function panics if there is no current reactor set, if the `rt` - /// feature flag is not enabled, or if thread-local runtime is not set. + /// This function panics if it is not called from within a runtime with + /// IO enabled or if the thread-local runtime is not set. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set From 990fb3a64726e4c36ca80d366a276f3b07a35f22 Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Mon, 18 Jul 2022 10:18:14 +0200 Subject: [PATCH 5/7] Updated panic description Changed to remove duplicate cause statements based on suggestions during the PR review. --- tokio/src/net/tcp/listener.rs | 2 +- tokio/src/net/tcp/stream.rs | 2 +- tokio/src/net/unix/datagram/socket.rs | 2 +- tokio/src/net/unix/listener.rs | 4 ++-- tokio/src/net/unix/stream.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tokio/src/net/tcp/listener.rs b/tokio/src/net/tcp/listener.rs index ea893f41079..c7389a69046 100644 --- a/tokio/src/net/tcp/listener.rs +++ b/tokio/src/net/tcp/listener.rs @@ -217,7 +217,7 @@ impl TcpListener { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/tcp/stream.rs b/tokio/src/net/tcp/stream.rs index 1337d07185c..1bd238b88f2 100644 --- a/tokio/src/net/tcp/stream.rs +++ b/tokio/src/net/tcp/stream.rs @@ -182,7 +182,7 @@ impl TcpStream { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/datagram/socket.rs b/tokio/src/net/unix/datagram/socket.rs index 963b20f3fb2..09d6fc1c1ec 100644 --- a/tokio/src/net/unix/datagram/socket.rs +++ b/tokio/src/net/unix/datagram/socket.rs @@ -431,7 +431,7 @@ impl UnixDatagram { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a Tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/listener.rs b/tokio/src/net/unix/listener.rs index f0106eefe4f..fbea3e76a1a 100644 --- a/tokio/src/net/unix/listener.rs +++ b/tokio/src/net/unix/listener.rs @@ -55,7 +55,7 @@ impl UnixListener { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set @@ -80,7 +80,7 @@ impl UnixListener { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set diff --git a/tokio/src/net/unix/stream.rs b/tokio/src/net/unix/stream.rs index f11025e76d3..82cec77fa87 100644 --- a/tokio/src/net/unix/stream.rs +++ b/tokio/src/net/unix/stream.rs @@ -700,7 +700,7 @@ impl UnixStream { /// # Panics /// /// This function panics if it is not called from within a runtime with - /// IO enabled or if the thread-local runtime is not set. + /// IO enabled. /// /// The runtime is usually set implicitly when this function is called /// from a future driven by a tokio runtime, otherwise runtime can be set From 73c5aea6cae0c60bd15e0ae4c4d8ec20a66e45ac Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Mon, 18 Jul 2022 14:05:46 +0200 Subject: [PATCH 6/7] Only test udp_socket_from_std_panic_caller when not on easi Co-authored-by: Alice Ryhl --- tokio/tests/net_panic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tokio/tests/net_panic.rs b/tokio/tests/net_panic.rs index a81ef479b1c..381862f833e 100644 --- a/tokio/tests/net_panic.rs +++ b/tokio/tests/net_panic.rs @@ -11,6 +11,7 @@ mod support { use support::panic::test_panic; #[test] +#[cfg(not(target_os = "wasi"))] fn udp_socket_from_std_panic_caller() -> Result<(), Box> { use std::net::SocketAddr; use tokio::net::UdpSocket; From 373c17adc28cd8ac2fb912c48613f553babf0d38 Mon Sep 17 00:00:00 2001 From: "Stainsby, Hayden" Date: Mon, 18 Jul 2022 22:32:15 +0200 Subject: [PATCH 7/7] Ignore all net_panic tests when target is wasi Catch unwind isn't supported, so these tests won't work. --- tokio/tests/net_panic.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tokio/tests/net_panic.rs b/tokio/tests/net_panic.rs index 381862f833e..d2d80ef5020 100644 --- a/tokio/tests/net_panic.rs +++ b/tokio/tests/net_panic.rs @@ -1,5 +1,5 @@ #![warn(rust_2018_idioms)] -#![cfg(feature = "full")] +#![cfg(all(feature = "full", not(target_os = "wasi")))] use std::error::Error; use tokio::net::{TcpListener, TcpStream}; @@ -11,7 +11,6 @@ mod support { use support::panic::test_panic; #[test] -#[cfg(not(target_os = "wasi"))] fn udp_socket_from_std_panic_caller() -> Result<(), Box> { use std::net::SocketAddr; use tokio::net::UdpSocket;