Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

task: add panic string to debug/display output of JoinError #6753

Merged
merged 5 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions tokio/src/runtime/task/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,18 @@ impl fmt::Display for JoinError {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.repr {
Repr::Cancelled => write!(fmt, "task {} was cancelled", self.id),
Repr::Panic(_) => write!(fmt, "task {} panicked", self.id),
Repr::Panic(p) => match panic_payload_as_str(p) {
Some(panic_str) => {
write!(
fmt,
"task {} panicked with message {:?}",
self.id, panic_str
)
}
None => {
write!(fmt, "task {} panicked", self.id)
}
},
}
}
}
Expand All @@ -149,7 +160,12 @@ impl fmt::Debug for JoinError {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.repr {
Repr::Cancelled => write!(fmt, "JoinError::Cancelled({:?})", self.id),
Repr::Panic(_) => write!(fmt, "JoinError::Panic({:?}, ...)", self.id),
Repr::Panic(p) => match panic_payload_as_str(p) {
Some(panic_str) => {
write!(fmt, "JoinError::Panic({:?}, {:?}, ...)", self.id, panic_str)
}
None => write!(fmt, "JoinError::Panic({:?}, ...)", self.id),
},
}
}
}
Expand All @@ -167,3 +183,20 @@ impl From<JoinError> for io::Error {
)
}
}

fn panic_payload_as_str(payload: &SyncWrapper<Box<dyn Any + Send>>) -> Option<&str> {
// Panic payloads are almost always `String` (if invoked with formatting arguments)
// or `&'static str` (if invoked with a string literal).
//
// Non-string panic payloads have niche use-cases,
// so we don't really need to worry about those.
if let Some(s) = payload.downcast_ref_sync::<String>() {
return Some(s);
}

if let Some(s) = payload.downcast_ref_sync::<&'static str>() {
return Some(s);
}

None
}
11 changes: 11 additions & 0 deletions tokio/src/util/sync_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//!
//! A similar primitive is provided in the `sync_wrapper` crate.

use std::any::Any;

pub(crate) struct SyncWrapper<T> {
value: T,
}
Expand All @@ -24,3 +26,12 @@ impl<T> SyncWrapper<T> {
self.value
}
}

impl SyncWrapper<Box<dyn Any + Send>> {
/// Attempt to downcast using `Any::downcast_ref()` to a type that is known to be `Sync`.
pub(crate) fn downcast_ref_sync<T: Any + Sync>(&self) -> Option<&T> {
// SAFETY: if the downcast fails, the inner value is not touched,
// so no thread-safety violation can occur.
self.value.downcast_ref()
}
}
110 changes: 110 additions & 0 deletions tokio/tests/task_abort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,113 @@ fn test_abort_task_that_panics_on_drop_returned() {
assert!(handle.await.unwrap_err().is_panic());
});
}

// It's not clear where these tests belong. This was the place suggested by @Darksonn:
// https://github.com/tokio-rs/tokio/pull/6753#issuecomment-2271434176
/// Checks that a `JoinError` with a panic payload prints the expected text.
#[test]
#[cfg(panic = "unwind")]
fn test_join_error_display() {
let rt = Builder::new_current_thread().build().unwrap();

rt.block_on(async move {
// `String` payload
let join_err = tokio::spawn(async move {
let value = 1234;
panic!("Format-args payload: {}", value)
})
.await
.unwrap_err();

// We can't assert the full output because the task ID can change.
let join_err_str = join_err.to_string();

assert!(
join_err_str.starts_with("task ")
&& join_err_str.ends_with(" panicked with message \"Format-args payload: 1234\""),
"Unexpected join_err_str {:?}",
join_err_str
);

// `&'static str` payload
let join_err = tokio::spawn(async move { panic!("Const payload") })
.await
.unwrap_err();

let join_err_str = join_err.to_string();

assert!(
join_err_str.starts_with("task ")
&& join_err_str.ends_with(" panicked with message \"Const payload\""),
"Unexpected join_err_str {:?}",
join_err_str
);

// Non-string payload
let join_err = tokio::spawn(async move { std::panic::panic_any(1234i32) })
.await
.unwrap_err();

let join_err_str = join_err.to_string();

assert!(
join_err_str.starts_with("task ") && join_err_str.ends_with(" panicked"),
"Unexpected join_err_str {:?}",
join_err_str
);
});
}

/// Checks that a `JoinError` with a panic payload prints the expected text from `Debug`.
#[test]
#[cfg(panic = "unwind")]
fn test_join_error_debug() {
let rt = Builder::new_current_thread().build().unwrap();

rt.block_on(async move {
// `String` payload
let join_err = tokio::spawn(async move {
let value = 1234;
panic!("Format-args payload: {}", value)
})
.await
.unwrap_err();

// We can't assert the full output because the task ID can change.
let join_err_str = format!("{:?}", join_err);

assert!(
join_err_str.starts_with("JoinError::Panic(Id(")
&& join_err_str.ends_with("), \"Format-args payload: 1234\", ...)"),
"Unexpected join_err_str {:?}",
join_err_str
);

// `&'static str` payload
let join_err = tokio::spawn(async move { panic!("Const payload") })
.await
.unwrap_err();

let join_err_str = format!("{:?}", join_err);

assert!(
join_err_str.starts_with("JoinError::Panic(Id(")
&& join_err_str.ends_with("), \"Const payload\", ...)"),
"Unexpected join_err_str {:?}",
join_err_str
);

// Non-string payload
let join_err = tokio::spawn(async move { std::panic::panic_any(1234i32) })
.await
.unwrap_err();

let join_err_str = format!("{:?}", join_err);

assert!(
join_err_str.starts_with("JoinError::Panic(Id(") && join_err_str.ends_with("), ...)"),
"Unexpected join_err_str {:?}",
join_err_str
);
});
}
Loading