Skip to content

Commit

Permalink
net: 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 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
  • Loading branch information
hds committed Jul 4, 2022
1 parent d8cad13 commit 90b3a27
Show file tree
Hide file tree
Showing 9 changed files with 244 additions and 6 deletions.
2 changes: 2 additions & 0 deletions tokio/src/io/poll_evented.rs
Expand Up @@ -77,6 +77,7 @@ impl<E: Source> PollEvented<E> {
/// 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<Self> {
PollEvented::new_with_interest(io, Interest::READABLE | Interest::WRITABLE)
Expand All @@ -97,6 +98,7 @@ impl<E: Source> PollEvented<E> {
/// 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> {
Self::new_with_interest_and_handle(io, interest, Handle::current())
Expand Down
4 changes: 3 additions & 1 deletion tokio/src/net/tcp/listener.rs
Expand Up @@ -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<TcpListener> {
let io = mio::net::TcpListener::from_std(listener);
let io = PollEvented::new(io)?;
Expand Down
4 changes: 3 additions & 1 deletion tokio/src/net/tcp/stream.rs
Expand Up @@ -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<TcpStream> {
let io = mio::net::TcpStream::from_std(stream);
let io = PollEvented::new(io)?;
Expand Down
2 changes: 2 additions & 0 deletions tokio/src/net/udp.rs
Expand Up @@ -170,6 +170,7 @@ impl UdpSocket {
UdpSocket::new(sys)
}

#[track_caller]
fn new(socket: mio::net::UdpSocket) -> io::Result<UdpSocket> {
let io = PollEvented::new(socket)?;
Ok(UdpSocket { io })
Expand Down Expand Up @@ -210,6 +211,7 @@ impl UdpSocket {
/// # Ok(())
/// # }
/// ```
#[track_caller]
pub fn from_std(socket: net::UdpSocket) -> io::Result<UdpSocket> {
let io = mio::net::UdpSocket::from_std(socket);
UdpSocket::new(io)
Expand Down
4 changes: 3 additions & 1 deletion tokio/src/net/unix/datagram/socket.rs
Expand Up @@ -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
Expand All @@ -457,6 +458,7 @@ impl UnixDatagram {
/// # Ok(())
/// # }
/// ```
#[track_caller]
pub fn from_std(datagram: net::UnixDatagram) -> io::Result<UnixDatagram> {
let socket = mio::net::UnixDatagram::from_std(datagram);
let io = PollEvented::new(socket)?;
Expand Down
8 changes: 6 additions & 2 deletions tokio/src/net/unix/listener.rs
Expand Up @@ -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<P>(path: P) -> io::Result<UnixListener>
where
P: AsRef<Path>,
Expand All @@ -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<UnixListener> {
let listener = mio::net::UnixListener::from_std(listener);
let io = PollEvented::new(listener)?;
Expand Down
4 changes: 3 additions & 1 deletion tokio/src/net/unix/stream.rs
Expand Up @@ -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<UnixStream> {
let stream = mio::net::UnixStream::from_std(stream);
let io = PollEvented::new(stream)?;
Expand Down
1 change: 1 addition & 0 deletions tokio/src/runtime/context.rs
Expand Up @@ -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();
Expand Down
221 changes: 221 additions & 0 deletions tokio/tests/net_panic.rs
@@ -0,0 +1,221 @@
#![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<dyn Error>> {
use std::net::SocketAddr;
use tokio::net::UdpSocket;

let addr = "127.0.0.1:8080".parse::<SocketAddr>().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<dyn Error>> {
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<dyn Error>> {
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<dyn Error>> {
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<dyn Error>> {
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<dyn Error>> {
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<dyn Error>> {
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<dyn Error>> {
use std::time::Duration;
use tokio::net::windows::named_pipe::ClientOptions;
use tokio::time;
use winapi::shared::winerror;

const PIPE_NAME: &str = r"\\.\pipe\named-pipe-idiomatic-client";

let rt = Builder::new_current_thread().enable_all().build().unwrap();
rt.block_on(async {
let server = tokio::spawn(async move {
loop {
// Wait for a client to connect.
let connected = server.connect().await?;

// Construct the next server to be connected before sending the one
// we already have of onto a task. This ensures that the server
// isn't closed (after it's done in the task) before a new one is
// available. Otherwise the client might error with
// `io::ErrorKind::NotFound`.
server = ServerOptions::new().create(PIPE_NAME)?;

let client = tokio::spawn(async move {
/* use the connected client */
Ok::<_, std::io::Error>(())
});
if true {
break;
} // needed for type inference to work
}

Ok::<_, io::Error>(())
});

let client = loop {
match ClientOptions::new().open(PIPE_NAME) {
Ok(client) => break client,
Err(e) if e.raw_os_error() == Some(winerror::ERROR_PIPE_BUSY as i32) => (),
Err(e) => return Err(e),
}

time::sleep(Duration::from_millis(50)).await;
};

client.max_instances(255);
});

Ok(())
}

// Runtime without `enable_io` so it has no IO driver set.
fn runtime_without_io() -> Runtime {
Builder::new_current_thread().build().unwrap()
}

0 comments on commit 90b3a27

Please sign in to comment.