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

[js] Avoid extraneous eprintln! invocation when callback is invoked from within a panic hook #240

Merged
merged 2 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions src/js/closures/common.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use ::core::ops::Not as _;

#[derive(Debug)]
pub
struct ArcClosureRawParts<CallFn> {
Expand Down
49 changes: 29 additions & 20 deletions src/js/closures/node_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
use super::*;

use ::napi::threadsafe_function::*;
use ::once_cell::sync::OnceCell;
use ::std::{
os::raw::c_void,
sync::Arc,
time::Duration
time::Duration,
};
use once_cell::sync::OnceCell;

/// Global configuration for the number of milliseconds to wait for a callback sync.
/// Global configuration for the number of milliseconds to wait for a callback sync.
/// Disabled when `None`. Access through `deadlock_timeout()`.
static DEADLOCK_TIMEOUT_MS: OnceCell<Option<u32>> = OnceCell::new();

Expand Down Expand Up @@ -561,14 +561,19 @@ macro_rules! impls {(
$_k: $_k, )*)?
) -> CRet
{
::scopeguard::defer_on_unwind! {
eprintln!("\
Attempted to panic through an `extern \"C\"` boundary, \
which is undefined behavior. \
Aborting for soundness.\
");
::std::process::abort();
}
// We set up an `on_unwind` guard, except if we're already being invoked
// from within a panicking context, which confuses the the `on_unwind` heuristic
danielhenrymantilla marked this conversation as resolved.
Show resolved Hide resolved
// of `::scopeguard`. Since in that case, any extra panic already triggers an abort,
// we can then just have the guard do nothing.
let _abort_on_unwind = ::std::thread::panicking().not().then(|| {
::scopeguard::guard_on_unwind((), |()| {
eprintln! {"\
Attempted to panic through the `extern \"C\"` boundary of a C `fn()`, \
which is undefined behavior. Aborting for soundness.\
"}
::std::process::abort();
})
});

let &Self {
ref ts_fun,
Expand Down Expand Up @@ -696,19 +701,23 @@ macro_rules! impls {(
);
debug_assert_eq!(status, Status::Ok);

let on_channel_closed = || panic!("Failed to receive a return value from a \
callback function (channel closed): missing `wrap_cb_for_ffi` call?");
let on_channel_closed = || panic! {"\
Failed to receive a return value from a callback function \
(channel closed): missing `wrap_cb_for_ffi` call?\
"};

if let Some(timeout) = get_deadlock_timeout() {
let duration = Duration::from_millis(timeout as u64);
match ret_receiver.recv_timeout(duration) {
Ok(ret) => ret,
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
panic!("Failed to receive a return value from a callback \
function (timeout after {}ms). This can be caused by a deadlock \
or when execution is interrupted by an interactive debugger. \
You can disable this check using \
`Ditto.disableDeadlockDetection()", timeout)
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => panic! {
"\
Failed to receive a return value from a callback function (timeout \
after {}ms). This can be caused by a deadlock or when execution is \
interrupted by an interactive debugger. You can disable this check \
using `Ditto.disableDeadlockDetection()\
",
timeout,
},
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
on_channel_closed()
Expand Down Expand Up @@ -843,7 +852,7 @@ fn dummy_ret_sender (env: &'_ Env)
}

/// Returns the configured deadlock timeout, or `None` if the timeout is disabled.
///
///
/// The timeout can be set once by calling `set_deadlock_timeout`. If that has not happened,
/// the default timeout is returned.
pub fn get_deadlock_timeout() -> Option<u32> {
Expand Down
22 changes: 12 additions & 10 deletions src/js/closures/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

use super::*;

use ::{
std::os::raw::c_void,
use ::std::{
os::raw::c_void,
};
use crate::{
layout::ReprC,
Expand Down Expand Up @@ -152,14 +152,16 @@ match_! {(
$_k: <$_k as ReprC>::CLayout )*)?
) -> <Ret as ReprC>::CLayout
{
::scopeguard::defer_on_unwind! {
eprintln!("\
Attempted to panic through an `extern \"C\"` boundary, \
which is undefined behavior. \
Aborting for soundness.\
");
::std::process::abort();
}
// Same logic as for `node_js`.
let _abort_on_unwind = ::std::thread::panicking().not().then(|| {
::scopeguard::guard_on_unwind((), |()| {
eprintln! {"\
Attempted to panic through the `extern \"C\"` boundary of a C `fn()`, \
which is undefined behavior. Aborting for soundness.\
"}
::std::process::abort();
})
});

let &Self {
ref js_fun,
Expand Down
Loading