Skip to content

Commit

Permalink
[js] Avoid extraneous eprintln! invocation when callback is invoked…
Browse files Browse the repository at this point in the history
… from within a panic hook (#240)

* Avoid extraneous `eprintln!` invocation when js callback is invoked from within a panic hook

* Typo
  • Loading branch information
danielhenrymantilla authored Sep 23, 2024
1 parent 501b1d4 commit f9b7ebb
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 30 deletions.
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 `on_unwind` heuristic
// 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

0 comments on commit f9b7ebb

Please sign in to comment.