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

Exceptions thrown across boundaries behave differently on Win/Linux #1226

Closed
futscdav opened this issue May 8, 2023 · 2 comments
Closed

Comments

@futscdav
Copy link

futscdav commented May 8, 2023

Consider this simple example of having a rust print function that calls a repr function on passed objects. As repr can be arbitrary Js code, it could throw an exception. Since the exception occurs inside the Rust -> Js -> Rust -> Js chain, the only plausible course of action is to return control to the Js engine via normal means and hope that it escapes from there into the intended TryCatch handler.

use v8;

fn call_object_property<'s>(
        scope: &mut v8::HandleScope<'s>, 
        value: v8::Local<v8::Value>,
        property: &str) -> Option<v8::Local<'s, v8::Value>>  {
    let object: v8::Local<v8::Object> = value.try_into().unwrap();
    let ident = v8::String::new(scope, property).unwrap().into();
    let prop = object.get(scope, ident).unwrap();
    let func: v8::Local<v8::Function> = prop.try_into().unwrap();
    let recv = scope.get_current_context().global(scope).into();

    let retval = func.call(scope, recv, &[]);
    return retval
}

fn print_fn(scope: &mut v8::HandleScope,
    args: v8::FunctionCallbackArguments,
    _retval: v8::ReturnValue) {
    let local_arg = args.get(0);
    let print_str = if local_arg.is_string() {
        local_arg.to_rust_string_lossy(scope)
    } else if local_arg.is_object() {
        let obj_repr = call_object_property(scope, local_arg, "repr");
        if obj_repr.is_none() {
            return;
        }
        "[".to_owned() + &obj_repr.unwrap().to_rust_string_lossy(scope) + "]"
    } else {
        "Unknown type".to_owned()
    };
    println!("{print_str}");
}

fn main() {
    let platform = v8::new_default_platform(0, false).make_shared();
    v8::V8::initialize_platform(platform);
    v8::V8::initialize();
    
    let mut isolate = v8::Isolate::new(v8::CreateParams::default());
    let scope = &mut v8::HandleScope::new(&mut isolate);
    let local_context = v8::Context::new(scope);
    let scope = &mut v8::ContextScope::new(scope, local_context);

    let global_proxy = scope.get_current_context().global(scope);

    let identifier = v8::String::new(scope, "print").unwrap();
    let value = v8::FunctionTemplate::new(scope, print_fn).get_function(scope).unwrap();
    global_proxy.set(scope, identifier.into(), value.into());

    let code = r#"
    let object_with_ok_repr = {
        repr: function() { return "object_with_ok_repr"; }
    };
    print(object_with_ok_repr);
    let object_with_broken_repr = {
        repr: function() { boom }
    };
    print(object_with_broken_repr);
    print("this should not be reachable");
    "#;

    let source = v8::String::new(scope, code).unwrap();
    let script = v8::Script::compile(scope, source, None).unwrap();

    let scope = &mut v8::TryCatch::new(scope);
    let _result = script.run(scope);
    assert!(scope.has_caught());
}

This happens as expected on my Windows machine. The script prints out

[object_with_ok_repr]

and exits. However, running on my Ubuntu desktop, returning control to Js results in continuation of execution and I get

[object_with_ok_repr]
this should not be reachable

The TryCatch is aware of the exception. Setting up another TryCatch inside the printing function and rethrowing in case of errors works as expected.

Unfortunately I don't have a working C++ v8 build on both of my machines to test whether this is an upstream issue or related to how the binding works. I'm aware that it's illegal to execute any Js operations while an exception is pending, but I don't see how you could avoid it here unless you are setting up another explicit TryCatch and rethrowing.

@bartlomieju
Copy link
Member

I can reliably repeat this on Mac. The problem is manifested only in debug build. On release build the second message is not printed. Feels like it might be related to denoland/deno#19021

bartlomieju added a commit that referenced this issue May 12, 2023
…1227)

Reproduces #1226 and denoland/deno#19021

```
// Fails
$ V8_FROM_SOURCE=1 cargo test exception
// Passes
$ V8_FROM_SOURCE=1 cargo test --release exception
```

We bisected this and this problem first appeared with V8 v11.2 upgrade. After further
bisects we established that v8/v8@1f349da#diff-de75fc3e7b84373d94e18b4531827e8b749f9bbe05b59707e894e4e0ce2a1535
is the first V8 commit that causes this failure. However after more investigation we can't
find any reason why that particular commit might cause this problem.

It is only reproducible in debug build, but not release build. Current working theory
is that it is a Rust compiler bug as changing the optimization level for this code
makes the bug go away. This commit should be considered a band-aid that works
around the problem, but doesn't fix it completely. We are gonna go with it as it
unblocks un on day-to-day work, but ultimately we should track it down (or wait
for another Rust upgrade which might fix it?).

---------

Co-authored-by: Bert Belder <bertbelder@gmail.com>
@bartlomieju
Copy link
Member

Thanks for the report. This is now fixed in 6ac0fab and I'm cutting 0.71.1 release which will include that fix.

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

No branches or pull requests

2 participants