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

Add Isolate::set_prepare_stack_trace_callback() #594

Conversation

bnoordhuis
Copy link
Contributor

@piscisaureus
Copy link
Member

@bnoordhuis

The crash on windows is due to the way non-POD (using C++03'd definition of POD) values are returned in the Win64 ABI.
Rust does not support this calling convention, but a workaround is possible.

Additionally, I prefer to move the unsafe CallbackScope creation to be done automatically in the trampoline.

The code snippet below takes care of both.

/// `PrepareStackTraceCallback` is called when the stack property of an error is
/// first accessed. The return value will be used as the stack value. If this
/// callback is registed, the `Error.prepareStackTrace()` API will be disabled.
///  `sites` is an array of call sites, specified in
/// https://v8.dev/docs/stack-trace-api.
///
/// # Example
///
/// ```
/// # use rusty_v8 as v8;
/// #
/// fn prepare_stack_trace_callback_example<'s>(
///   scope: &mut v8::HandleScope<'s>,
///   error: v8::Local<'s, v8::Value>,
///   sites: v8::Local<'s, v8::Array>,
/// ) -> Option<v8::Local<'s, v8::Value>> {
///   todo!()
/// }
/// ```
pub trait PrepareStackTraceCallback:
  UnitType
  + for<'s> FnOnce(
    &mut HandleScope<'s>,
    Local<'s, Value>,
    Local<'s, Array>,
  ) -> Option<Local<'s, Value>>
{
}

impl<F> PrepareStackTraceCallback for F where
  F: UnitType
    + for<'s> FnOnce(
      &mut HandleScope<'s>,
      Local<'s, Value>,
      Local<'s, Array>,
    ) -> Option<Local<'s, Value>>
{
}

#[macro_export]
macro_rules! impl_prepare_stack_trace_callback {
  () => {
    impl $crate::PrepareStackTraceCallback
    + for<'__s> ::std::ops::FnOnce(
      &mut $crate::HandleScope<'__s>,
      $crate::Local<'__s, $crate::Value>,
      $crate::Local<'__s, $crate::Array>,
    ) -> ::std::option::Option<$crate::Local<'__s, $crate::Value>>
  };
}

#[cfg(target_family = "unix")]
#[repr(transparent)]
pub(crate) struct RawPrepareStackTraceCallback(
  for<'s> extern "C" fn(
    Local<'s, Context>,
    Local<'s, Value>,
    Local<'s, Array>,
  ) -> Option<Local<'s, Value>>,
);

#[cfg(all(target_family = "windows", target_arch = "x86_64"))]
#[repr(transparent)]
pub(crate) struct RawPrepareStackTraceCallback(
  for<'s> extern "C" fn(
    *mut Option<Local<'s, Value>>,
    Local<'s, Context>,
    Local<'s, Value>,
    Local<'s, Array>,
  ) -> *mut Option<Local<'s, Value>>,
);

impl<F: PrepareStackTraceCallback> From<F> for RawPrepareStackTraceCallback {
  fn from(_: F) -> Self {
    #[inline(always)]
    fn signature_adapter<'s, F: PrepareStackTraceCallback>(
      context: Local<'s, Context>,
      error: Local<'s, Value>,
      sites: Local<'s, Array>,
    ) -> Option<Local<'s, Value>> {
      let scope = &mut unsafe { CallbackScope::new(context) };
      (F::get())(scope, error, sites)
    }

    #[cfg(target_family = "unix")]
    #[inline(always)]
    extern "C" fn abi_adapter<'s, F: PrepareStackTraceCallback>(
      context: Local<'s, Context>,
      error: Local<'s, Value>,
      sites: Local<'s, Array>,
    ) -> Option<Local<'s, Value>> {
      signature_adapter::<F>(context, error, sites)
    }

    #[cfg(all(target_family = "windows", target_arch = "x86_64"))]
    #[inline(always)]
    extern "C" fn abi_adapter<'s, F: PrepareStackTraceCallback>(
      return_value: *mut Option<Local<'s, Value>>,
      context: Local<'s, Context>,
      error: Local<'s, Value>,
      sites: Local<'s, Array>,
    ) -> *mut Option<Local<'s, Value>> {
      unsafe {
        std::ptr::write(
          return_value,
          signature_adapter::<F>(context, error, sites),
        );
        return_value
      }
    }

    Self(abi_adapter::<F>)
  }
}

#[cfg(test)]
fn mock_prepare_stack_trace_callback<'s>(
  _scope: &mut HandleScope<'s>,
  _error: Local<'s, Value>,
  _sites: Local<'s, Array>,
) -> Option<Local<'s, Value>> {
  unimplemented!()
}

#[test]
fn prepare_stack_trace_callback_as_type_param() {
  fn pass_as_type_param<F: PrepareStackTraceCallback>(
    _: F,
  ) -> RawPrepareStackTraceCallback {
    F::get().into()
  }
  let _ = pass_as_type_param(mock_prepare_stack_trace_callback);
}

#[test]
fn prepare_stack_trace_callback_as_impl_trait() {
  fn pass_as_impl_trait(
    f: impl PrepareStackTraceCallback,
  ) -> RawPrepareStackTraceCallback {
    f.into()
  }
  let _ = pass_as_impl_trait(mock_prepare_stack_trace_callback);
}

#[test]
fn prepare_stack_trace_callback_as_impl_macro() {
  fn pass_as_impl_macro(
    f: impl_prepare_stack_trace_callback!(),
  ) -> RawPrepareStackTraceCallback {
    f.into()
  }
  let _ = pass_as_impl_macro(mock_prepare_stack_trace_callback);
  let _ = pass_as_impl_macro(|_scope, _error, _sites| unimplemented!());
}

@bnoordhuis bnoordhuis force-pushed the isolate-set_prepare_stack_trace_callback branch from 93576c9 to 8a78355 Compare February 7, 2021 13:07
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bnoordhuis bnoordhuis merged commit 64aa11e into denoland:master Feb 8, 2021
@bnoordhuis bnoordhuis deleted the isolate-set_prepare_stack_trace_callback branch February 8, 2021 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants