servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.
Mozilla Public License 2.0
293 stars 122 forks source link

Cannot create second runtime. #442

Open jeremyjh opened 5 years ago

jeremyjh commented 5 years ago

It seems Runtime::new can be called successfully only once on a thread.

I get the Err result calling it a second time. This is on 0.9.2 on nightly-2018-10-12 on MacOS High Sierra.

Running this test passes if assign() is called once, fails when called twice.

#[macro_use]
extern crate mozjs;
extern crate libc;

use mozjs::jsapi::CompartmentOptions;
use mozjs::jsapi::JS_NewGlobalObject;
use mozjs::jsapi::OnNewGlobalHookOption;
use mozjs::jsval::UndefinedValue;
use mozjs::rust::{Runtime, SIMPLE_GLOBAL_CLASS};

use std::ptr;

#[test]
fn test_assign() {
    assign();
    assign();
}

fn assign() {
    let runtime = Runtime::new().unwrap();
    let context = runtime.cx();
    let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook;
    let c_option = CompartmentOptions::default();

    unsafe {
        let global = JS_NewGlobalObject(context, &SIMPLE_GLOBAL_CLASS, ptr::null_mut(), h_option, &c_option);
        rooted!(in(context) let global_root = global);
        let global = global_root.handle();
        let javascript = "var x = 5;";
        rooted!(in(context) let mut rval = UndefinedValue());
        let _ = runtime.evaluate_script(global, javascript, "test.js", 0, rval.handle_mut());
    }
}
running 1 test
test test_assign ... FAILED

failures:

---- test_assign stdout ----
thread 'test_assign' panicked at 'called `Result::unwrap()` on an `Err` value: ()', libcore/result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
jdm commented 5 years ago

This is caused by this code.

jeremyjh commented 5 years ago

This is caused by this code.

Would it not be safe to create a new runtime on the second invocation? It seems the direct culprit is here, but why is that needed?

jdm commented 5 years ago

This is why we can't just start up the runtime again when we've previously called JS_ShutDown.

jeremyjh commented 5 years ago

I'm good with that as a constraint but I do need to be able to create multiple runtimes, that should be possible right?

I tried adding this as the first line to the test so that the runtime count does not drop to 0 but that results in a SEGV.

     let _runtime = Runtime::new().unwrap();
jdm commented 5 years ago

Which is not to say that I would not accept pull requests to address this issue; just that https://github.com/servo/rust-mozjs/pull/344 was introduced to ensure that the runtime did actually shut down when there is no clear synchronization point when multiple threads are in use.

jdm commented 5 years ago

The easiest way to deal with your situation would be to create a global runtime that outlives every other one.

EDIT: which I see you did... that shouldn't crash :<