Issue: SIGSEGV on consecutive runs via unit tests

[jasonterando]

Your library is saving my sanity. I'm writing a webservice test utility and want to integrate a BDD-style test capability using JavaScript. js-sandbox is working splendidly. Except... I'm getting a weird behavior when unit testing and I'm not sure if it's something I should be worried about. Further down is a simplified code example demonstrating the issue.

I'm using "0.2.0-rc.2" on Linux. When executing more than one unit test for js-sandbox via cargo test, I am getting a SIGSEGV crash. I don't know if this something I should be worried about, or if it just an oddity with cargo test that I should just workaround.

Thanks for providing this library. I'll hopefully be in a position to "pay it forward" soon...

$ cargo test
   Compiling demo-js v0.1.0 (/mnt/data/projects/Personal/apicize/rust/@apicize/demo-js)
    Finished test [unoptimized + debuginfo] target(s) in 2.09s
     Running unittests src/lib.rs (target/debug/deps/demo_js-bd1e1ddd656d6fba)

running 2 tests
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/mnt/data/projects/Personal/apicize/rust/@apicize/demo-js/target/debug/deps/demo_js-bd1e1ddd656d6fba` (signal: 11, SIGSEGV: invalid memory reference)
jterando@TerandoOffice:/mnt/data/projects/Personal/apicize/rust/@apicize/demo-js$ 

What is odd is that if I comment out the second unit test, which does exactly the same as the first, I get a successful run.

$ cargo test
   Compiling demo-js v0.1.0 (/mnt/data/projects/Personal/apicize/rust/@apicize/demo-js)
    Finished test [unoptimized + debuginfo] target(s) in 2.14s
     Running unittests src/lib.rs (target/debug/deps/demo_js-bd1e1ddd656d6fba)

running 1 test
test tests::test1 ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests demo-js

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Am I doing something wrong? Should I even worry about this?

src/lib.rs

use js_sandbox::{JsError, Script};

pub fn execute() -> Result<i32, JsError> {
    let mut js_code = String::new();
    js_code.push_str(r###"
        function sum(values) {
            return values.reduce((sum, v) => sum + v, 0);
        }
    "###);
    let script = Script::from_string(js_code.as_str());
    if let Err(err) = script {
        return Err(err);
    }
    let v = [1, 2];
    let result: Result<i32, JsError> =
        script.unwrap().call("sum", (&v,));
    
    match result {
        Ok(results) => Ok(results),
        Err(err) => Err(err),
    }
}


#[cfg(test)]
mod tests {
    use crate::execute;

    #[test]
    fn test1() {
        assert_eq!(execute().unwrap(), 3)
    }

    // Including test2 causes (signal: 11, SIGSEGV: invalid memory reference) when running "cargo test",
    // commenting it out allows cargo test to complete succesfully
    #[test]
    fn test2() {
        assert_eq!(execute().unwrap(), 3)
    }
}

Cargo.toml

[package]
name = "demo-js"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
js-sandbox = "0.2.0-rc.2"

[Bromeon]

Thanks for reporting! The js-sandbox crate does not use unsafe, so it cannot (in its own code) create undefined behavior or segmentation faults. It's very likely that this occurs with one of our dependencies, probably deno_core or v8.

Maybe we should first try to update the deno_core version (v0.209.0 -> v0.240.0). Unfortunately, this seems to contain again several breaking changes -- without deprecations, migration guides or documentation of any kind. It has been a bit frustrating to keep up with Deno, and I can't currently spare the time for busywork like this 😕

If this can be reproduced with the latest deno_core version, it might be worth reporting it upstream. UB is definitely something to worry about -- if it can occur in a test, it can also likely occur in production.

[jasonterando]

Thanks. For now I'm going to "brute force" using V8 directly using JSON serialization/deserialization with serde. When I get a bit more rust-fu under my belt, I'll revisit and maybe look into whether we can transplant Deno's serialization/deserialization and use that without the "rest" of Deno.

[jasonterando]

FWIW, I was able to reproduce the issue with rusty v8, no Deno or js-sandbox involved. I've submitted an issue here if you want to track progress.

[jasonterando]

So, by default, V8 needs to be run in the main thread. If you are going to call it from another thread, you have to set up the platform using new_unprotected_default_platform. That's why cargo test bombed out - because it launches a new thread for tests.

[Bromeon]

I see. Thanks for reporting and getting back so quickly!

Do you think there's a way to set up our tests so they honor the main thread for setup? 🤔

[jasonterando]

Maybe define an environment variable (V8_TEST) that, if set, would trigger creating the platform using new_unprotected_default_platform? We'd set that environment variable in something like an .env used for testing.