denoland / deno_core

The core engine at the heart of Deno
MIT License
265 stars 85 forks source link

segmentation fault when running multiple runtimes #150

Open jsen- opened 1 year ago

jsen- commented 1 year ago

I was trying to run multiple instances of deno_core::JsRuntime within one tokio task, but got:

    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/test1`
[Ok(()), Ok(())]
[Ok(()), Ok(())]
[Ok(()), Ok(())]
Segmentation fault (core dumped)
# Cargo.toml
[package]
name = "test1"
version = "0.1.0"
edition = "2021"

[dependencies]
deno_core = "0.202.0"
futures = "0.3.28"
tokio = "1.31.0"
// main.rs
use deno_core::{error::AnyError, op, Extension, ModuleCode, ModuleSpecifier, Op};

#[op]
async fn op_sleep_ms() {
    // not await-ing here works fine
    tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}

pub async fn run(name: String, module: ModuleCode) -> Result<(), AnyError> {
    let extensions = Extension {
        ops: std::borrow::Cow::Borrowed(&[op_sleep_ms::DECL]),
        ..Default::default()
    };
    let mut js_runtime = deno_core::JsRuntime::new(deno_core::RuntimeOptions {
        extensions: vec![extensions],
        ..Default::default()
    });

    let module_path = ModuleSpecifier::from_file_path(name).unwrap();
    let mod_id = js_runtime
        .load_main_module(&module_path, Some(module))
        .await?;
    let result = js_runtime.mod_evaluate(mod_id);

    js_runtime.run_event_loop(false).await?;
    result.await?
}

async fn real_main() -> Result<(), AnyError> {
    loop {
        // 0..1 works fine
        let tasks = (0..2)
            .map(|i| {
                run(
                    format!("/just_sleep_{i}.mjs"),
                    ModuleCode::Static("Deno.core.ops.op_sleep_ms().await"),
                )
            })
            .collect::<Vec<_>>();
        println!("{:?}", futures::future::join_all(tasks).await);
    }
}

fn main() {
    tokio::runtime::Builder::new_current_thread()
        .enable_io()
        .enable_time()
        .build()
        .unwrap()
        .block_on(real_main())
        .unwrap();
}

tested on: rustc 1.71.1 (eb26296b5 2023-08-03) and rustc 1.73.0-nightly (1b198b3a1 2023-08-13)

the fault occurs in v8IsolateNew()

Is this supposed to work? Is there a good other way to run multiple completely isolated scripts in parallel? (I'd prefer to avoid creating new thread per runtime, however running multiple scripts/modules within the same runtime would be fine, as long as each has a separate Global object)

mmastrac commented 1 year ago

I'm not aware of any v8 limitations on multiple isolates in one thread, but I would strongly recommend against it as we may end up internalizing the the tokio runtime inside of the JsRuntime for performance reasons at some time in the future.

JsRealm is intended to separate scripts from each other (they use a v8::Context internally), but realms are still in development and may have some sharp edges w.r.t. performance and security. For now, the best supported way to run parallel isolates is one-per-thread.

jsen- commented 1 year ago

Thank you for the JsRealm tip. I don't see the performance nor security as an issue (all scripts will be written internally and will mostly be waiting for API responses). There will be 0 - 300 of them running in parallel, though. I tried it and it works, but the implementation is kinda crazy :slightly_smiling_face: and I'm still missing the ability to "pause" and "cancel" individual JsRealms.

Feel free to close this issue (unless you'd like to keep it open with regards to the segfault). Thanks again

abadcafe commented 1 year ago

@jsen- I think your problem maybe related to this issue: https://github.com/denoland/rusty_v8/issues/639 .

The root cause is, v8::Isolate::new() included a call of v8::Isolate->Enter() and the drop() method of Isolate included a call of v8::Isolate->Exit(), which required isolates in a thread must destruct in last-in-first-out order.

If this is your case, then the simplest way to fix it is just use FuturesOrdered and its push_front() method to ensure JsRuntimes destruct in reversed order of create. But I recommend you just remove the Enter() and Exit() pair in rusty_v8 source code, as it has no sense in single thread embedding situation.

And as a deno_core user, I really don't like the idea to encapsulate tokio and thread-related things into deno_core. This way will limit the usage of deno_core in a very narrow scope. I beg you core developers to re-consider this idea @mmastrac .

spolu commented 2 months ago

We at https://github.com/dust-tt/dust are running in the same issues. We would like to run isolated pieces of JS code running deno but moving to recent version of deno we run into seg faults as the JsRuntime gets dropped.

The natural next step was to move to one jsRuntime shared through channels but it requires isolatioon and it seems that the Realms API was removed?

rscarson commented 2 months ago

Just popping in to 2nd @abadcafe

Inlining the tokio runtime into JsRuntime would definitely narrow the core's possible uses and hurt downstream crates

kckeiks commented 5 days ago

Same issue here. We tried using FuturesOrdered but that didn't work. For reference https://github.com/denoland/rusty_v8/issues/1628