From b918b8a66043f20136a57f1e04c3db8ed4ffb2af Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Fri, 19 Jun 2026 16:44:22 +1000 Subject: [PATCH 1/4] feat: add global MiniRacer pause gate Add MiniRacer.pause/resume to quiesce operations process-wide with timeout handling and nested pauses. Expose PauseTimeoutError and opt-in Process._fork hooks so fork can wait for MiniRacer to drain before parent and child continue. Document the fork coordination APIs and cover pause, timeout, hook, and single-threaded fork behavior with tests. --- .github/workflows/ci.yml | 5 +- CHANGELOG | 1 + README.md | 39 +- .../mini_racer_extension.c | 566 ++++++++++++++++-- lib/mini_racer.rb | 62 ++ test/mini_racer_test.rb | 259 ++++++++ test/single_threaded_test.rb | 109 ++++ 7 files changed, 985 insertions(+), 56 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6384515..fa27058 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: matrix: os: - "macos-latest" - - "ubuntu-latest" + - "ubuntu-24.04" ruby: - "truffleruby+graalvm" @@ -39,6 +39,9 @@ jobs: runs-on: ${{ matrix.os }} timeout-minutes: 10 + env: + TRUFFLERUBYOPT: "--jvm --polyglot" + steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4 with: diff --git a/CHANGELOG b/CHANGELOG index 2a4b8cd..b8abecb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ - Avoid finalizer hangs when a forked child garbage-collects a non-idle inherited `:single_threaded` context - Allow Ruby thread interrupts, process shutdown, and cross-thread `Context#dispose` to terminate busy `:single_threaded` JavaScript execution instead of hanging - Make `Context#dispose` while an attached Ruby callback is active either terminate safely or raise instead of deadlocking + - Add `MiniRacer.pause(timeout:)` / `MiniRacer.resume` to quiesce MiniRacer globally, plus opt-in fork hooks built on that pause gate - 0.21.2 - 11-06-2026 - Add `Context#perform_microtask_checkpoint` to synchronously drain the V8 microtask queue, useful for spec-compliant `dispatchEvent` sequencing inside Ruby callbacks diff --git a/README.md b/README.md index 5e2faa3..f96278e 100644 --- a/README.md +++ b/README.md @@ -143,10 +143,41 @@ When using pre-fork `MiniRacer::Context` objects in `:single_threaded` mode, ensure the process only forks while MiniRacer is quiescent: no thread may be evaluating JavaScript, calling into a context, disposing/freeing a context, running a Ruby callback from JavaScript, or otherwise using MiniRacer at the -instant of `fork`. In multi-threaded applications, guard all MiniRacer context -operations and the `fork` itself with the same application-level lock. Forking -while a MiniRacer operation is in progress can leave inherited pthread mutexes -in an unusable state in the child process. +instant of `fork`. Forking while a MiniRacer operation is in progress can leave +inherited pthread mutexes in an unusable state in the child process. + +`MiniRacer.pause(timeout:)` is a process-global quiesce gate. It prevents new +MiniRacer operations from starting, waits for operations already in progress to +finish, and then keeps MiniRacer paused until `MiniRacer.resume` is called. +`timeout:` is in seconds; if MiniRacer cannot drain in time, +`MiniRacer::PauseTimeoutError` is raised and the pause is rolled back. Omitting +`timeout:` waits indefinitely, which is useful only when the caller knows active +JavaScript cannot get stuck. + +```ruby +MiniRacer.pause(timeout: 5) +begin + pid = fork do + MiniRacer.resume # child: reset inherited pause state + # child process work + end +ensure + MiniRacer.resume # parent: release the pause +end +``` + +For normal Ruby forks you can install an opt-in `Process._fork` hook which uses +that same pause gate automatically: + +```ruby +MiniRacer.install_fork_hooks!(timeout: 5) +``` + +The hook covers `Kernel#fork`, `Process.fork`, and `IO.popen("-")` on Rubies +that expose `Process._fork`. It intentionally does not cover `Process.daemon` or +raw native `fork(2)` calls from other C extensions. When the hook is installed, +do not call `MiniRacer.resume` again in the child block; the hook already resumes +in both parent and child before user child code runs. If you want to ensure your application does not leak memory after fork either: diff --git a/ext/mini_racer_extension/mini_racer_extension.c b/ext/mini_racer_extension/mini_racer_extension.c index 14dec60..25cd044 100644 --- a/ext/mini_racer_extension/mini_racer_extension.c +++ b/ext/mini_racer_extension/mini_racer_extension.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #if defined(__linux__) && !defined(__GLIBC__) @@ -192,6 +193,7 @@ static const rb_data_type_t snapshot_type = { static VALUE platform_init_error; static VALUE context_disposed_error; +static VALUE pause_timeout_error; static VALUE parse_error; static VALUE memory_error; static VALUE script_error; @@ -208,6 +210,219 @@ static VALUE js_function_class; static pthread_mutex_t flags_mtx = PTHREAD_MUTEX_INITIALIZER; static Buf flags; // protected by |flags_mtx| +#if defined(__GNUC__) || defined(__clang__) +static __thread int mini_racer_operation_depth; +#else +static _Thread_local int mini_racer_operation_depth; +#endif + +#ifndef __APPLE__ +#define MINI_RACER_PAUSE_CLOCK CLOCK_MONOTONIC +#else +#define MINI_RACER_PAUSE_CLOCK CLOCK_REALTIME +#endif + +#define MINI_RACER_MAX_PAUSE_TIMEOUT (10.0 * 365.0 * 24.0 * 60.0 * 60.0) + +typedef struct MiniRacerPauseState +{ + pthread_mutex_t mtx; + pthread_cond_t cv; + atomic_int pause_depth; + atomic_int active; + atomic_long pid; +} MiniRacerPauseState; + +static MiniRacerPauseState pause_state = { + .mtx = PTHREAD_MUTEX_INITIALIZER, + .pause_depth = 0, + .active = 0, +}; + +struct mini_racer_gate_wait +{ + atomic_int cancel; + int *counted; +}; + +struct mini_racer_pause_wait +{ + atomic_int cancel; + int timed; + int active; + struct timespec deadline; +}; + +static void mini_racer_pause_state_init(int reset_mutex) +{ + pthread_condattr_t cattr; + + if (reset_mutex) { + // Forked children inherit the bytes of parent pthread mutexes/conds, + // but not the parent threads that may have owned or waited on them. + // POSIX is not kind to reinitializing an already-initialized object; + // this mirrors Ruby's pragmatic atfork approach and avoids touching + // parent-owned synchronization state in the child. + pthread_mutex_init(&pause_state.mtx, NULL); + } + pthread_condattr_init(&cattr); +#ifndef __APPLE__ + pthread_condattr_setclock(&cattr, MINI_RACER_PAUSE_CLOCK); +#endif + pthread_cond_init(&pause_state.cv, &cattr); + pthread_condattr_destroy(&cattr); + atomic_store(&pause_state.pause_depth, 0); + atomic_store(&pause_state.active, 0); + atomic_store(&pause_state.pid, (long)getpid()); + mini_racer_operation_depth = 0; +} + +static inline void mini_racer_pause_recover_after_fork(void) +{ + if (atomic_load(&pause_state.pid) != (long)getpid()) + mini_racer_pause_state_init(1); +} + +static void mini_racer_pause_wakeup_all(void) +{ + mini_racer_pause_recover_after_fork(); + pthread_mutex_lock(&pause_state.mtx); + pthread_cond_broadcast(&pause_state.cv); + pthread_mutex_unlock(&pause_state.mtx); +} + +static void mini_racer_timespec_from_timeout(struct timespec *ts, double timeout) +{ + double seconds; + long nsec; + + clock_gettime(MINI_RACER_PAUSE_CLOCK, ts); + nsec = (long)(modf(timeout, &seconds) * 1000000000.0); + ts->tv_sec += (time_t)seconds; + ts->tv_nsec += nsec; + if (ts->tv_nsec >= 1000000000L) { + ts->tv_sec++; + ts->tv_nsec -= 1000000000L; + } +} + +static const char *mini_racer_error_message(int r) +{ + if (r == ECANCELED) + return "MiniRacer operation was interrupted or canceled"; + return strerror(r); +} + +static int mini_racer_operation_wait_unpaused(atomic_int *cancel, atomic_int *interrupted) +{ + int r; + + if ((r = pthread_mutex_lock(&pause_state.mtx))) + return r; + while (atomic_load(&pause_state.pause_depth) > 0 && + !atomic_load(cancel) && + !(interrupted && atomic_load(interrupted))) { + if ((r = pthread_cond_wait(&pause_state.cv, &pause_state.mtx))) { + pthread_mutex_unlock(&pause_state.mtx); + return r; + } + } + pthread_mutex_unlock(&pause_state.mtx); + if (atomic_load(cancel)) + return ECANCELED; + if (interrupted && atomic_load(interrupted)) + return EINTR; + return 0; +} + +static void mini_racer_active_decrement(void) +{ + if (atomic_fetch_sub(&pause_state.active, 1) == 1 && + atomic_load(&pause_state.pause_depth) > 0) { + pthread_mutex_lock(&pause_state.mtx); + pthread_cond_broadcast(&pause_state.cv); + pthread_mutex_unlock(&pause_state.mtx); + } +} + +static int mini_racer_operation_enter_nogvl(atomic_int *cancel, atomic_int *interrupted, int *counted) +{ + int r; + + *counted = 0; + if (mini_racer_operation_depth > 0) { + mini_racer_operation_depth++; + return 0; + } + + mini_racer_pause_recover_after_fork(); + for (;;) { + if (atomic_load(cancel)) + return ECANCELED; + if (interrupted && atomic_load(interrupted)) + return EINTR; + if (atomic_load(&pause_state.pause_depth) == 0) { + atomic_fetch_add(&pause_state.active, 1); + if (atomic_load(&pause_state.pause_depth) == 0) { + mini_racer_operation_depth = 1; + *counted = 1; + return 0; + } + mini_racer_active_decrement(); + } + if ((r = mini_racer_operation_wait_unpaused(cancel, interrupted))) + return r; + } +} + +static void mini_racer_operation_leave_nogvl(int counted) +{ + if (mini_racer_operation_depth > 0) + mini_racer_operation_depth--; + if (!counted || mini_racer_operation_depth > 0) + return; + + mini_racer_pause_recover_after_fork(); + mini_racer_active_decrement(); +} + +static void *mini_racer_operation_enter_nogvl_entry(void *arg) +{ + struct mini_racer_gate_wait *w; + int r; + + w = arg; + r = mini_racer_operation_enter_nogvl(&w->cancel, NULL, w->counted); + return (void *)(intptr_t)r; +} + +static void mini_racer_gate_wait_ubf(void *arg) +{ + struct mini_racer_gate_wait *w; + + w = arg; + atomic_store(&w->cancel, 1); + mini_racer_pause_wakeup_all(); +} + +static int mini_racer_operation_enter_gvl(int *counted) +{ + struct mini_racer_gate_wait w; + void *r; + + atomic_init(&w.cancel, 0); + *counted = 0; + w.counted = counted; + r = rb_nogvl(mini_racer_operation_enter_nogvl_entry, &w, + mini_racer_gate_wait_ubf, &w, 0); + return (int)(intptr_t)r; +} + +static void mini_racer_operation_leave_gvl(int counted) +{ + mini_racer_operation_leave_nogvl(counted); +} + // arg == &(struct rendezvous_nogvl){...} static void *rendezvous_callback(void *arg); @@ -234,7 +449,8 @@ struct rendezvous_nogvl Buf *req, *res; atomic_int active; atomic_int interrupted; - int started, finished, has_rr_mtx; + atomic_int cancel; + int started, finished, has_rr_mtx, operation_entered, counted; }; struct rendezvous_des @@ -1073,6 +1289,11 @@ static int single_threaded_recover_after_fork(Context *c) #ifndef __APPLE__ pthread_condattr_setclock(&cattr, CLOCK_MONOTONIC); #endif + // In a forked child, the runner thread and any waiters from the parent no + // longer exist. Reinitialize the inherited condition variable in place + // rather than destroying it; this is technically outside POSIX's happy path + // for already-initialized condvars, but it avoids touching parent-owned + // waiter state and matches Ruby's pragmatic atfork reset style. r = pthread_cond_init(&c->cv, &cattr); pthread_condattr_destroy(&cattr); if (r) @@ -1104,12 +1325,17 @@ static void rendezvous_release(struct rendezvous_nogvl *a) Context *c; atomic_store(&a->active, 0); - if (!a->has_rr_mtx) - return; - c = a->context; - c->depth--; - a->has_rr_mtx = 0; - pthread_mutex_unlock(&c->rr_mtx); + if (a->has_rr_mtx) { + c = a->context; + c->depth--; + a->has_rr_mtx = 0; + pthread_mutex_unlock(&c->rr_mtx); + } + if (a->operation_entered) { + mini_racer_operation_leave_nogvl(a->counted); + a->operation_entered = 0; + a->counted = 0; + } } static inline void *rendezvous_nogvl(void *arg) @@ -1120,10 +1346,22 @@ static inline void *rendezvous_nogvl(void *arg) a = arg; c = a->context; + if (!a->operation_entered) { + if ((r = mini_racer_operation_enter_nogvl(&a->cancel, &a->interrupted, &a->counted))) + return (void *)(intptr_t)r; + a->operation_entered = 1; + } if (!a->started) { - if (single_threaded && (r = single_threaded_recover_after_fork(c))) + if (single_threaded && (r = single_threaded_recover_after_fork(c))) { + rendezvous_release(a); return (void *)(intptr_t)r; + } pthread_mutex_lock(&c->rr_mtx); + if (atomic_load(&a->cancel)) { + pthread_mutex_unlock(&c->rr_mtx); + rendezvous_release(a); + return (void *)(intptr_t)ECANCELED; + } a->has_rr_mtx = 1; if (c->depth > 0 && c->depth%50 == 0) { // TODO stop steep recursion fprintf(stderr, "mini_racer: deep js->ruby->js recursion, depth=%d\n", c->depth); @@ -1136,7 +1374,7 @@ static inline void *rendezvous_nogvl(void *arg) next: atomic_store(&a->active, 1); pthread_mutex_lock(&c->mtx); - if (atomic_load(&c->quit)) { + if (atomic_load(&c->quit) || atomic_load(&a->cancel)) { buf_reset(a->req); pthread_mutex_unlock(&c->mtx); a->finished = 1; @@ -1159,14 +1397,18 @@ static inline void *rendezvous_nogvl(void *arg) } pthread_cond_signal(&c->cv); } - while (!c->res_ready && !atomic_load(&a->interrupted) && !atomic_load(&c->quit)) + while (!c->res_ready && + !atomic_load(&a->interrupted) && + !atomic_load(&a->cancel) && + !atomic_load(&c->quit)) { pthread_cond_wait(&c->cv, &c->mtx); + } if (!c->res_ready && atomic_load(&a->interrupted)) { atomic_store(&a->active, 0); pthread_mutex_unlock(&c->mtx); return (void *)(intptr_t)EINTR; } - if (!c->res_ready && atomic_load(&c->quit)) { + if (!c->res_ready && (atomic_load(&c->quit) || atomic_load(&a->cancel))) { buf_reset(a->req); pthread_mutex_unlock(&c->mtx); a->finished = 1; @@ -1181,7 +1423,7 @@ static inline void *rendezvous_nogvl(void *arg) if (*a->res->buf == 'c') { // js -> ruby callback? rb_thread_call_with_gvl(rendezvous_callback, a); buf_reset(a->res); - if (atomic_load(&c->quit)) { + if (atomic_load(&c->quit) || atomic_load(&a->cancel)) { buf_reset(a->req); a->finished = 1; rendezvous_release(a); @@ -1200,18 +1442,30 @@ static void rendezvous_ubf(void *arg) Context *c; a = arg; + atomic_store(&a->interrupted, 1); + mini_racer_pause_wakeup_all(); if (!atomic_load(&a->active)) return; - atomic_store(&a->interrupted, 1); c = a->context; pthread_cond_broadcast(&c->cv); } +struct context_dispose_wait +{ + Context *context; + atomic_int cancel; + int counted; +}; + static void terminate_ubf(void *arg) { + struct context_dispose_wait *a; Context *c; - c = arg; + a = arg; + atomic_store(&a->cancel, 1); + mini_racer_pause_wakeup_all(); + c = a->context; if (c->pst) v8_terminate_execution(c->pst); pthread_cond_broadcast(&c->cv); @@ -1226,6 +1480,8 @@ static void *rendezvous_cancel_nogvl(void *arg) Context *c; a = arg; + atomic_store(&a->cancel, 1); + mini_racer_pause_wakeup_all(); c = a->context; atomic_store(&a->active, 0); if (c->pst) @@ -1304,16 +1560,19 @@ static void rendezvous_no_des(Context *c, Buf *req, Buf *res) a.res = res; atomic_init(&a.active, 0); atomic_init(&a.interrupted, 0); + atomic_init(&a.cancel, 0); a.started = 0; a.finished = 0; a.has_rr_mtx = 0; + a.operation_entered = 0; + a.counted = 0; rv = rb_ensure(rendezvous_no_des_body, (VALUE)&a, rendezvous_no_des_ensure, (VALUE)&a); r = (void *)(intptr_t)NUM2LONG(rv); if ((int)(intptr_t)r == ECANCELED) rb_raise(context_disposed_error, "disposed context"); if (r) - rb_raise(runtime_error, "single-threaded runner: %s", strerror((int)(intptr_t)r)); + rb_raise(runtime_error, "MiniRacer operation: %s", mini_racer_error_message((int)(intptr_t)r)); } // send request to & receive reply from v8 thread; takes ownership of |req| @@ -1597,30 +1856,50 @@ static VALUE context_attach(VALUE self, VALUE name, VALUE proc) static void *context_dispose_do(void *arg) { + struct context_dispose_wait *a; Context *c; + void *ret; int r; - c = arg; + a = arg; + c = a->context; + ret = NULL; + if ((r = mini_racer_operation_enter_nogvl(&a->cancel, NULL, &a->counted))) + return (void *)(intptr_t)r; if (single_threaded) { - if ((r = single_threaded_recover_after_fork(c))) - return (void *)(intptr_t)r; + if ((r = single_threaded_recover_after_fork(c))) { + ret = (void *)(intptr_t)r; + goto out; + } + } + if (atomic_load(&a->cancel)) { + ret = (void *)(intptr_t)ECANCELED; + goto out; } if (c->depth > 0) { r = pthread_mutex_trylock(&c->rr_mtx); if (!r) { pthread_mutex_unlock(&c->rr_mtx); - return (void *)(intptr_t)EBUSY; + ret = (void *)(intptr_t)EBUSY; + goto out; + } + if (r != EBUSY) { + ret = (void *)(intptr_t)r; + goto out; } - if (r != EBUSY) - return (void *)(intptr_t)r; if (c->pst) v8_terminate_execution(c->pst); pthread_cond_broadcast(&c->cv); } if (single_threaded) { pthread_mutex_lock(&c->mtx); - while (c->req.len || c->res.len) + while ((c->req.len || c->res.len) && !atomic_load(&a->cancel)) pthread_cond_wait(&c->cv, &c->mtx); + if (atomic_load(&a->cancel)) { + pthread_mutex_unlock(&c->mtx); + ret = (void *)(intptr_t)ECANCELED; + goto out; + } atomic_store(&c->quit, 1); // disposed if (c->single_threaded_thr_started && c->single_threaded_pid == getpid()) { pthread_cond_signal(&c->cv); @@ -1632,24 +1911,35 @@ static void *context_dispose_do(void *arg) pthread_mutex_unlock(&c->mtx); } else { pthread_mutex_lock(&c->mtx); - while (c->req.len || c->res.len) + while ((c->req.len || c->res.len) && !atomic_load(&a->cancel)) pthread_cond_wait(&c->cv, &c->mtx); + if (atomic_load(&a->cancel)) { + pthread_mutex_unlock(&c->mtx); + ret = (void *)(intptr_t)ECANCELED; + goto out; + } atomic_store(&c->quit, 1); // disposed pthread_cond_signal(&c->cv); // wake up v8 thread pthread_mutex_unlock(&c->mtx); } - return NULL; +out: + mini_racer_operation_leave_nogvl(a->counted); + return ret; } static VALUE context_dispose(VALUE self) { Context *c; + struct context_dispose_wait a; void *r; TypedData_Get_Struct(self, Context, &context_type, c); - r = rb_thread_call_without_gvl(context_dispose_do, c, terminate_ubf, c); + a.context = c; + a.counted = 0; + atomic_init(&a.cancel, 0); + r = rb_thread_call_without_gvl(context_dispose_do, &a, terminate_ubf, &a); if (r) - rb_raise(runtime_error, "context dispose: %s", strerror((int)(intptr_t)r)); + rb_raise(runtime_error, "context dispose: %s", mini_racer_error_message((int)(intptr_t)r)); return Qnil; } @@ -1928,6 +2218,141 @@ static VALUE platform_set_flags(int argc, VALUE *argv, VALUE klass) rb_raise(platform_init_error, "platform already initialized"); } +static void *mini_racer_pause_nogvl(void *arg) +{ + struct mini_racer_pause_wait *w; + int r; + + w = arg; + mini_racer_pause_recover_after_fork(); + if ((r = pthread_mutex_lock(&pause_state.mtx))) + return (void *)(intptr_t)r; + atomic_fetch_add(&pause_state.pause_depth, 1); + for (;;) { + if (atomic_load(&w->cancel)) { + r = ECANCELED; + goto fail; + } + if (atomic_load(&pause_state.active) == 0) { + pthread_mutex_unlock(&pause_state.mtx); + return NULL; + } + if (w->timed) { + r = pthread_cond_timedwait(&pause_state.cv, &pause_state.mtx, &w->deadline); + if (r == ETIMEDOUT && atomic_load(&pause_state.active) == 0) + continue; + if (r) + goto fail; + } else if ((r = pthread_cond_wait(&pause_state.cv, &pause_state.mtx))) { + goto fail; + } + } +fail: + w->active = atomic_load(&pause_state.active); + if (atomic_fetch_sub(&pause_state.pause_depth, 1) == 1) + pthread_cond_broadcast(&pause_state.cv); + pthread_mutex_unlock(&pause_state.mtx); + return (void *)(intptr_t)r; +} + +static void mini_racer_pause_ubf(void *arg) +{ + struct mini_racer_pause_wait *w; + + w = arg; + atomic_store(&w->cancel, 1); + mini_racer_pause_wakeup_all(); +} + +static double mini_racer_parse_pause_timeout(int argc, VALUE *argv, int *timed) +{ + VALUE kwargs, vals[1]; + ID keys[1]; + double timeout; + + rb_scan_args(argc, argv, ":", &kwargs); + *timed = 0; + if (NIL_P(kwargs)) + return 0; + keys[0] = rb_intern("timeout"); + rb_get_kwargs(kwargs, keys, 0, 1, vals); + if (vals[0] == Qundef || NIL_P(vals[0])) + return 0; + if (!RTEST(rb_obj_is_kind_of(vals[0], rb_cNumeric))) + rb_raise(rb_eArgError, "timeout must be a number"); + timeout = NUM2DBL(vals[0]); + if (!isfinite(timeout) || timeout < 0 || timeout > MINI_RACER_MAX_PAUSE_TIMEOUT) + rb_raise(rb_eArgError, "timeout must be a finite number between 0 and 10 years"); + *timed = 1; + return timeout; +} + +static VALUE mini_racer_resume(VALUE self); + +static VALUE mini_racer_pause_yield(VALUE arg) +{ + (void)arg; + return rb_yield(Qnil); +} + +static VALUE mini_racer_pause_ensure_resume(VALUE self) +{ + int status; + + rb_protect(mini_racer_resume, self, &status); + if (status) + rb_set_errinfo(Qnil); + return Qnil; +} + +static VALUE mini_racer_pause(int argc, VALUE *argv, VALUE self) +{ + struct mini_racer_pause_wait w; + double timeout; + void *r; + + if (mini_racer_operation_depth > 0) + rb_raise(runtime_error, "cannot pause MiniRacer from inside an active MiniRacer operation"); + + timeout = mini_racer_parse_pause_timeout(argc, argv, &w.timed); + atomic_init(&w.cancel, 0); + w.active = 0; + if (w.timed) + mini_racer_timespec_from_timeout(&w.deadline, timeout); + r = rb_nogvl(mini_racer_pause_nogvl, &w, mini_racer_pause_ubf, &w, 0); + if (r) { + if ((int)(intptr_t)r == ETIMEDOUT) + rb_raise(pause_timeout_error, "MiniRacer.pause timed out waiting for %d active operation%s", + w.active, w.active == 1 ? "" : "s"); + rb_raise(runtime_error, "MiniRacer.pause: %s", mini_racer_error_message((int)(intptr_t)r)); + } + if (rb_block_given_p()) + return rb_ensure(mini_racer_pause_yield, Qnil, mini_racer_pause_ensure_resume, self); + return Qtrue; +} + +static VALUE mini_racer_resume(VALUE self) +{ + int depth, empty; + + (void)self; + if (atomic_load(&pause_state.pid) != (long)getpid()) { + mini_racer_pause_state_init(1); + return Qnil; + } + pthread_mutex_lock(&pause_state.mtx); + depth = atomic_load(&pause_state.pause_depth); + if (depth <= 0) { + pthread_mutex_unlock(&pause_state.mtx); + rb_raise(runtime_error, "MiniRacer.resume called without a matching pause"); + } + empty = (atomic_fetch_sub(&pause_state.pause_depth, 1) == 1); + if (empty) + pthread_cond_broadcast(&pause_state.cv); + pthread_mutex_unlock(&pause_state.mtx); + return Qnil; +} + // called by v8_global_init; caller must free |*p| with free() void v8_get_flags(char **p, size_t *n) { @@ -1950,16 +2375,68 @@ void v8_get_flags(char **p, size_t *n) rb_thread_lock_native_thread(); } -static VALUE context_initialize(int argc, VALUE *argv, VALUE self) +struct context_initialize_args { - VALUE kwargs, a, k, v; + Context *context; + int counted; +}; + +static VALUE context_initialize_do(VALUE arg) +{ + struct context_initialize_args *a; pthread_attr_t attr; const char *cause; pthread_t thr; + Context *c; + int r; + + a = (struct context_initialize_args *)arg; + c = a->context; + + cause = "MiniRacer operation"; + if ((r = mini_racer_operation_enter_gvl(&a->counted))) + goto fail; + if (single_threaded) { + v8_once_init(); + c->pst = v8_thread_init(c, c->snapshot.buf, c->snapshot.len, c->max_memory, c->verbose_exceptions); + } else { + cause = "pthread_attr_init"; + if ((r = pthread_attr_init(&attr))) + goto fail; + pthread_attr_setstacksize(&attr, 2<<20); // 2 MiB + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + // v8 thread takes ownership of |c| + cause = "pthread_create"; + r = pthread_create(&thr, &attr, v8_thread_start, c); + pthread_attr_destroy(&attr); + if (r) + goto fail; + barrier_wait(&c->early_init); + barrier_wait(&c->late_init); + } + return Qnil; +fail: + rb_raise(runtime_error, "Context.initialize: %s: %s", cause, mini_racer_error_message(r)); + return Qnil; // pacify compiler +} + +static VALUE context_initialize_ensure(VALUE arg) +{ + struct context_initialize_args *a; + + a = (struct context_initialize_args *)arg; + if (a->counted) + mini_racer_operation_leave_gvl(a->counted); + return Qnil; +} + +static VALUE context_initialize(int argc, VALUE *argv, VALUE self) +{ + VALUE kwargs, a, k, v; + struct context_initialize_args init_args; Snapshot *ss; Context *c; char *s; - int r; TypedData_Get_Struct(self, Context, &context_type, c); rb_scan_args(argc, argv, ":", &kwargs); @@ -2002,28 +2479,10 @@ static VALUE context_initialize(int argc, VALUE *argv, VALUE self) } } init: - if (single_threaded) { - v8_once_init(); - c->pst = v8_thread_init(c, c->snapshot.buf, c->snapshot.len, c->max_memory, c->verbose_exceptions); - } else { - cause = "pthread_attr_init"; - if ((r = pthread_attr_init(&attr))) - goto fail; - pthread_attr_setstacksize(&attr, 2<<20); // 2 MiB - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - // v8 thread takes ownership of |c| - cause = "pthread_create"; - r = pthread_create(&thr, &attr, v8_thread_start, c); - pthread_attr_destroy(&attr); - if (r) - goto fail; - barrier_wait(&c->early_init); - barrier_wait(&c->late_init); - } - return Qnil; -fail: - rb_raise(runtime_error, "Context.initialize: %s: %s", cause, strerror(r)); - return Qnil; // pacify compiler + init_args.context = c; + init_args.counted = 0; + return rb_ensure(context_initialize_do, (VALUE)&init_args, + context_initialize_ensure, (VALUE)&init_args); } static VALUE snapshot_alloc(VALUE klass) @@ -2157,10 +2616,15 @@ void Init_mini_racer_extension(void) VALUE c, m; m = rb_define_module("MiniRacer"); + mini_racer_pause_state_init(0); c = rb_define_class_under(m, "Error", rb_eStandardError); snapshot_error = rb_define_class_under(m, "SnapshotError", c); platform_init_error = rb_define_class_under(m, "PlatformAlreadyInitialized", c); context_disposed_error = rb_define_class_under(m, "ContextDisposedError", c); + pause_timeout_error = rb_define_class_under(m, "PauseTimeoutError", c); + + rb_define_singleton_method(m, "pause", mini_racer_pause, -1); + rb_define_singleton_method(m, "resume", mini_racer_resume, 0); c = rb_define_class_under(m, "EvalError", c); parse_error = rb_define_class_under(m, "ParseError", c); diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 0687abd..3d2c218 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -47,6 +47,8 @@ class ContextDisposedError < Error end class PlatformAlreadyInitialized < Error end + class PauseTimeoutError < Error + end class EvalError < Error end @@ -70,6 +72,66 @@ def backtrace end end + module ForkHooks + def _fork + paused = false + MiniRacer.pause(timeout: MiniRacer.fork_hook_timeout) + paused = true + + super + ensure + exception = $! + if paused + begin + MiniRacer.resume + rescue StandardError + # Keep the original fork/pause failure. + raise unless exception + end + end + end + end + private_constant :ForkHooks + + @fork_hook_timeout = 5.0 + @fork_hooks_installed = false + MAX_FORK_HOOK_TIMEOUT = 10 * 365 * 24 * 60 * 60 + private_constant :MAX_FORK_HOOK_TIMEOUT + + class << self + attr_reader :fork_hook_timeout + + def install_fork_hooks!(timeout: 5.0) + unless respond_to?(:pause) && respond_to?(:resume) + raise NotImplementedError, + "MiniRacer.pause/resume fork coordination is not available on this platform" + end + unless Process.respond_to?(:_fork, true) + raise NotImplementedError, + "Process._fork is not available on this platform" + end + unless timeout.nil? + unless timeout.is_a?(Numeric) + raise ArgumentError, + "timeout must be nil or a finite number between 0 and 10 years" + end + timeout = timeout.to_f + unless timeout.finite? && timeout >= 0 && + timeout <= MAX_FORK_HOOK_TIMEOUT + raise ArgumentError, + "timeout must be nil or a finite number between 0 and 10 years" + end + end + + @fork_hook_timeout = timeout + unless @fork_hooks_installed + Process.singleton_class.prepend(ForkHooks) + @fork_hooks_installed = true + end + true + end + end + class ScriptError < EvalError def initialize(message) message, *@frames = message.split("\n") diff --git a/test/mini_racer_test.rb b/test/mini_racer_test.rb index e9ff5c5..db8985e 100644 --- a/test/mini_racer_test.rb +++ b/test/mini_racer_test.rb @@ -91,6 +91,263 @@ def test_that_it_has_a_version_number refute_nil ::MiniRacer::VERSION end + def test_pause_blocks_new_operations_until_resume + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + context = MiniRacer::Context.new + paused = false + result = Queue.new + thread = nil + + MiniRacer.pause(timeout: 1) + paused = true + thread = Thread.new { result << context.eval("1 + 1") } + sleep 0.1 + + assert thread.alive?, "eval should wait while MiniRacer is paused" + assert result.empty?, "eval should not finish while MiniRacer is paused" + + MiniRacer.resume + paused = false + + assert_equal 2, result.pop + assert thread.join(3), "eval did not finish after MiniRacer.resume" + ensure + begin + MiniRacer.resume if paused + rescue StandardError + nil + end + thread&.kill if thread&.alive? + thread&.join + end + + def test_pause_times_out_while_operation_is_active + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + started_r, started_w = IO.pipe + release_r, release_w = IO.pipe + context = MiniRacer::Context.new + context.attach( + "block", + proc do + started_w.write("x") + started_w.flush + release_r.read(1) + 42 + end + ) + + worker = Thread.new { context.eval("block()") } + started_r.read(1) + + paused = false + begin + MiniRacer.pause(timeout: 0.05) + paused = true + flunk "MiniRacer.pause should time out while an operation is active" + rescue MiniRacer::PauseTimeoutError + # expected + ensure + begin + MiniRacer.resume if paused + rescue StandardError + nil + end + end + + release_w.write("x") + release_w.flush + assert worker.join(3), "active eval did not finish" + assert_equal 2, context.eval("1 + 1") + ensure + begin + release_w&.write("x") + rescue StandardError + nil + end + worker&.kill if worker&.alive? + worker&.join + [started_r, started_w, release_r, release_w].each do |io| + begin + io&.close + rescue StandardError + nil + end + end + end + + def test_pause_block_resumes_after_exception + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + assert_raises(::RuntimeError) do + MiniRacer.pause(timeout: 1) { raise "boom" } + end + + assert_equal 2, MiniRacer::Context.new.eval("1 + 1") + end + + def test_pause_allows_reentrant_callback_work_to_drain + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + started_r, started_w = IO.pipe + release_r, release_w = IO.pipe + context = MiniRacer::Context.new + context.attach( + "reenter", + proc do + started_w.write("x") + started_w.flush + release_r.read(1) + context.eval("20 + 22") + end + ) + + worker = Thread.new { context.eval("reenter()") } + started_r.read(1) + + paused = false + pause_thread = + Thread.new do + MiniRacer.pause(timeout: 1) + paused = true + end + + sleep 0.1 + release_w.write("x") + release_w.flush + + assert pause_thread.join(3), + "pause did not wait for reentrant callback work to drain" + assert worker.join(3), "eval did not finish" + assert_equal 42, worker.value + ensure + begin + MiniRacer.resume if paused + rescue StandardError + nil + end + begin + release_w&.write("x") + rescue StandardError + nil + end + worker&.kill if worker&.alive? + pause_thread&.kill if pause_thread&.alive? + worker&.join + pause_thread&.join + [started_r, started_w, release_r, release_w].each do |io| + begin + io&.close + rescue StandardError + nil + end + end + end + + def test_pause_block_preserves_original_exception_if_block_resumes + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + error = + assert_raises(::RuntimeError) do + MiniRacer.pause(timeout: 1) do + MiniRacer.resume + raise "original" + end + end + assert_equal "original", error.message + end + + def test_pause_is_nested_until_all_resumes_run + thread = nil + paused = 0 + + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + context = MiniRacer::Context.new + result = Queue.new + + MiniRacer.pause(timeout: 1) + paused += 1 + MiniRacer.pause(timeout: 1) + paused += 1 + + thread = Thread.new { result << context.eval("20 + 22") } + sleep 0.1 + assert thread.alive?, "eval should wait while nested pause is held" + + MiniRacer.resume + paused -= 1 + sleep 0.1 + assert thread.alive?, "eval should still wait until the outer pause resumes" + + MiniRacer.resume + paused -= 1 + assert_equal 42, result.pop + assert thread.join(3), "eval did not finish after outer resume" + ensure + paused.times do + begin + MiniRacer.resume + rescue StandardError + nil + end + end + thread&.kill if thread&.alive? + thread&.join + end + + def test_resume_without_pause_raises + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + assert_raises(MiniRacer::RuntimeError) { MiniRacer.resume } + end + + def test_pause_rejects_invalid_timeout + unless MiniRacer.respond_to?(:pause) + skip "MiniRacer.pause is only implemented for CRuby" + end + + assert_raises(ArgumentError) { MiniRacer.pause(timeout: -1) } + assert_raises(ArgumentError) { MiniRacer.pause(timeout: Float::NAN) } + assert_raises(ArgumentError) { MiniRacer.pause(timeout: Float::INFINITY) } + assert_raises(ArgumentError) { MiniRacer.pause(timeout: "5") } + assert_raises(ArgumentError) { MiniRacer.pause(timeout: 1e300) } + end + + def test_fork_hooks_reject_invalid_timeout + unless MiniRacer.respond_to?(:install_fork_hooks!) && + Process.respond_to?(:_fork, true) + skip "MiniRacer.install_fork_hooks! is only implemented for CRuby" + end + + assert_raises(ArgumentError) { MiniRacer.install_fork_hooks!(timeout: -1) } + assert_raises(ArgumentError) do + MiniRacer.install_fork_hooks!(timeout: Float::NAN) + end + assert_raises(ArgumentError) do + MiniRacer.install_fork_hooks!(timeout: Float::INFINITY) + end + assert_raises(ArgumentError) { MiniRacer.install_fork_hooks!(timeout: "5") } + assert_raises(ArgumentError) do + MiniRacer.install_fork_hooks!(timeout: 1e300) + end + end + def test_types context = MiniRacer::Context.new assert_equal 2, context.eval("2") @@ -1610,6 +1867,8 @@ def test_termination_exception sleep 1.5 a.kill b.kill + assert a.join(3), "stop thread did not stop" + assert b.join(3), "heap stats thread did not stop" end def test_ruby_exception diff --git a/test/single_threaded_test.rb b/test/single_threaded_test.rb index 60ac7aa..9598b3b 100644 --- a/test/single_threaded_test.rb +++ b/test/single_threaded_test.rb @@ -388,4 +388,113 @@ def test_fork_after_low_memory_notification raise "child failed with status #{status.inspect}" unless status.success? RUBY end + + def test_fork_hook_pauses_and_recovers_child + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + MiniRacer.install_fork_hooks!(timeout: 1) + + context = MiniRacer::Context.new + context.eval("var answer = 41") + context.eval("answer += 1") + + pid = fork do + Thread.new do + sleep 3 + warn "child timed out" + exit! 99 + end + + exit!(context.eval("answer") == 42 ? 0 : 1) + end + _, status = Process.wait2(pid) + raise "child failed with status #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + end + + def test_manual_pause_resume_around_fork + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + context = MiniRacer::Context.new + context.eval("var answer = 41") + context.eval("answer += 1") + + MiniRacer.pause(timeout: 1) + begin + pid = fork do + Thread.new do + sleep 3 + warn "child timed out" + exit! 99 + end + + MiniRacer.resume + exit!(context.eval("answer") == 42 ? 0 : 1) + end + ensure + MiniRacer.resume + end + + _, status = Process.wait2(pid) + raise "child failed with status #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + end + + def test_fork_hook_times_out_instead_of_forking_while_busy + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + MiniRacer.install_fork_hooks!(timeout: 0.05) + + started_r, started_w = IO.pipe + release_r, release_w = IO.pipe + context = MiniRacer::Context.new + context.attach("block", proc do + started_w.write("x") + started_w.flush + release_r.read(1) + 42 + end) + + worker = Thread.new { context.eval("block()") } + started_r.read(1) + + begin + fork { exit! 88 } + raise "expected pause timeout" + rescue MiniRacer::PauseTimeoutError + end + + release_w.write("x") + release_w.flush + raise "worker did not finish" unless worker.join(3) + raise "context should still be usable" unless context.eval("1 + 1") == 2 + RUBY + end + + def test_fork_hook_rejects_fork_from_active_callback + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + MiniRacer.install_fork_hooks!(timeout: 1) + + context = MiniRacer::Context.new + context.attach("try_fork", proc do + begin + fork { exit! 88 } + "forked" + rescue MiniRacer::RuntimeError => e + raise unless e.message.include?("cannot pause") + "rejected" + end + end) + + raise "fork was not rejected" unless context.eval("try_fork()") == "rejected" + raise "context should still be usable" unless context.eval("1 + 1") == 2 + RUBY + end end From 81546771b08588c60f3b132991a2426a045f8bcd Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 13 Aug 2026 11:24:47 +1000 Subject: [PATCH 2/4] FIX: preserve non-exception jump tags during deserialization --- ext/mini_racer_extension/mini_racer_extension.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ext/mini_racer_extension/mini_racer_extension.c b/ext/mini_racer_extension/mini_racer_extension.c index 25cd044..a70ef92 100644 --- a/ext/mini_racer_extension/mini_racer_extension.c +++ b/ext/mini_racer_extension/mini_racer_extension.c @@ -1594,11 +1594,8 @@ static VALUE rendezvous1(Context *c, Buf *req, DesCtx *d) } r = rb_protect(deserialize, (VALUE)&(struct rendezvous_des){d, &res}, &exc); buf_reset(&res); - if (exc) { - r = rb_errinfo(); - rb_set_errinfo(Qnil); - rb_exc_raise(r); - } + if (exc) + rb_jump_tag(exc); return r; } From 62f8531e707a8b7bab316c3d45665a41c3bc88f5 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 13 Aug 2026 13:20:55 +1000 Subject: [PATCH 3/4] fix: reject unsafe V8 use after fork Track V8 platform ownership and context state across forks, and raise MiniRacer::ForkError when a child attempts to use unrecoverable inherited state. Keep quiescent single-threaded contexts usable while rejecting busy ones. Warn when fork hooks are used with the default platform, and document why pause hooks cannot restore its missing worker threads. Add coverage for platform initialization races, inherited context cleanup, and safe child initialization. --- CHANGELOG | 1 + README.md | 41 ++- .../mini_racer_extension.c | 311 +++++++++++++++-- ext/mini_racer_extension/mini_racer_v8.cc | 11 +- ext/mini_racer_extension/mini_racer_v8.h | 6 +- lib/mini_racer.rb | 30 ++ test/fork_safety_test.rb | 330 ++++++++++++++++++ test/single_threaded_test.rb | 75 ++++ 8 files changed, 760 insertions(+), 45 deletions(-) create mode 100644 test/fork_safety_test.rb diff --git a/CHANGELOG b/CHANGELOG index b8abecb..717fa85 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ - Allow Ruby thread interrupts, process shutdown, and cross-thread `Context#dispose` to terminate busy `:single_threaded` JavaScript execution instead of hanging - Make `Context#dispose` while an attached Ruby callback is active either terminate safely or raise instead of deadlocking - Add `MiniRacer.pause(timeout:)` / `MiniRacer.resume` to quiesce MiniRacer globally, plus opt-in fork hooks built on that pause gate + - Detect and reject use of an inherited default V8 platform or context in forked children, where V8's missing worker threads cannot be recovered; add `MiniRacer::ForkError` and fork-hook guidance to use `:single_threaded` - 0.21.2 - 11-06-2026 - Add `Context#perform_microtask_checkpoint` to synchronously drain the V8 microtask queue, useful for spec-compliant `dispatchEvent` sequencing inside Ruby callbacks diff --git a/README.md b/README.md index f96278e..9873300 100644 --- a/README.md +++ b/README.md @@ -133,18 +133,28 @@ context.eval("bar()", filename: "a/bar.js") Some Ruby web servers employ forking (for example unicorn or puma in clustered mode). V8 is not fork safe by default and sadly Ruby does not have support for fork notifications per [#5446](https://bugs.ruby-lang.org/issues/5446). -Since 0.6.1 mini_racer does support V8 single threaded platform mode which should remove most forking related issues. To enable run this before using `MiniRacer::Context`, for example in a Rails initializer: +Since 0.6.1 mini_racer supports V8's single-threaded platform mode, which is +required when MiniRacer is initialized before a fork and will be used in the +child. Enable it before creating any `MiniRacer::Context` or +`MiniRacer::Snapshot`, for example in a Rails initializer: ```ruby MiniRacer::Platform.set_flags!(:single_threaded) ``` +V8's default platform creates a process-global worker pool. If that platform is +initialized before `fork`, the child inherits its synchronization state but not +its worker threads. The child therefore cannot safely use inherited contexts or +repair the problem by creating a new context. MiniRacer detects such use and +raises `MiniRacer::ForkError`. Default mode remains valid when V8 is first +initialized after the fork, or when the child never uses MiniRacer. + When using pre-fork `MiniRacer::Context` objects in `:single_threaded` mode, ensure the process only forks while MiniRacer is quiescent: no thread may be evaluating JavaScript, calling into a context, disposing/freeing a context, running a Ruby callback from JavaScript, or otherwise using MiniRacer at the instant of `fork`. Forking while a MiniRacer operation is in progress can leave -inherited pthread mutexes in an unusable state in the child process. +inherited pthread mutexes in an unusable state. `MiniRacer.pause(timeout:)` is a process-global quiesce gate. It prevents new MiniRacer operations from starting, waits for operations already in progress to @@ -152,7 +162,11 @@ finish, and then keeps MiniRacer paused until `MiniRacer.resume` is called. `timeout:` is in seconds; if MiniRacer cannot drain in time, `MiniRacer::PauseTimeoutError` is raised and the pause is rolled back. Omitting `timeout:` waits indefinitely, which is useful only when the caller knows active -JavaScript cannot get stuck. +JavaScript cannot get stuck. The gate coordinates MiniRacer operations; it does +not stop or rebuild the default platform's V8 worker pool, so a successful pause +does not make a default platform initialized before fork reusable in the child. + +For pre-initialized `:single_threaded` contexts, manual coordination looks like: ```ruby MiniRacer.pause(timeout: 5) @@ -167,17 +181,22 @@ end ``` For normal Ruby forks you can install an opt-in `Process._fork` hook which uses -that same pause gate automatically: +that same pause gate automatically. Applications that preload MiniRacer and use +it in forked children should configure both pieces before creating contexts: ```ruby +MiniRacer::Platform.set_flags!(:single_threaded) MiniRacer.install_fork_hooks!(timeout: 5) ``` -The hook covers `Kernel#fork`, `Process.fork`, and `IO.popen("-")` on Rubies -that expose `Process._fork`. It intentionally does not cover `Process.daemon` or -raw native `fork(2)` calls from other C extensions. When the hook is installed, -do not call `MiniRacer.resume` again in the child block; the hook already resumes -in both parent and child before user child code runs. +Installing hooks while the default platform is configured emits an advisory +warning, because the hook alone cannot make a pre-initialized default worker pool +safe in the child. The hook covers `Kernel#fork`, `Process.fork`, and +`IO.popen("-")` on Rubies that expose `Process._fork`. It intentionally does not +cover `Process.daemon` or raw native `fork(2)` calls from other C extensions. +When the hook is installed, do not call `MiniRacer.resume` again in the child +block; the hook already resumes in both parent and child before user child code +runs. If you want to ensure your application does not leak memory after fork either: @@ -193,6 +212,10 @@ ObjectSpace.each_object(MiniRacer::Context){|c| c.dispose} # fork here ``` +Disposing contexts does not uninitialize V8's process-global platform. If the +default platform was already initialized, disposal avoids context leaks but does +not permit the child to use MiniRacer; use `:single_threaded` for that case. + ### Threadsafe Context usage is threadsafe diff --git a/ext/mini_racer_extension/mini_racer_extension.c b/ext/mini_racer_extension/mini_racer_extension.c index a70ef92..dd55a91 100644 --- a/ext/mini_racer_extension/mini_racer_extension.c +++ b/ext/mini_racer_extension/mini_racer_extension.c @@ -64,10 +64,37 @@ static inline void rb_thread_lock_native_thread(void) #define countof(x) (sizeof(x) / sizeof(*(x))) #define endof(x) ((x) + countof(x)) -// mostly RO: assigned once by platform_set_flag1 while holding |flags_mtx|, -// from then on read-only and accessible without holding locks +// mostly RO: assigned by platform_set_flag1 while holding |flags_mtx| before +// V8 initialization, then read-only after v8_get_flags consumes the flags int single_threaded; +enum MiniRacerPlatformMode { + MINI_RACER_PLATFORM_DEFAULT = 0, + MINI_RACER_PLATFORM_SINGLE_THREADED = 1, +}; + +enum MiniRacerV8InitPhase { + MINI_RACER_V8_UNINITIALIZED = 0, + MINI_RACER_V8_STARTING = 1, + MINI_RACER_V8_COMPLETE = 2, +}; + +#define MINI_RACER_FORK_CONTEXT_ERROR 0x4d01 +#define MINI_RACER_FORK_PLATFORM_ERROR 0x4d02 +#define MINI_RACER_FORK_BUSY_ERROR 0x4d03 +#define MINI_RACER_FORK_INIT_ERROR 0x4d04 + +// Packed into one atomic so a forked child cannot observe a phase, mode, and +// owner pid from different initialization states. V8 builds supported by this +// extension run on platforms with lock-free 64-bit atomics. +typedef uint_fast64_t MiniRacerV8InitState; +static atomic_uint_fast64_t v8_init_state; +static pthread_once_t v8_once = PTHREAD_ONCE_INIT; + +#define MINI_RACER_V8_STATE_PHASE_MASK ((MiniRacerV8InitState)0x3) +#define MINI_RACER_V8_STATE_MODE_SHIFT 2 +#define MINI_RACER_V8_STATE_PID_SHIFT 3 + // work around missing pthread_barrier_t on macOS typedef struct Barrier { @@ -133,6 +160,13 @@ typedef struct Context // gets too complicated atomic_int quit; int verbose_exceptions; + // Context initialization/lifecycle ownership. |platform_mode| is frozen + // before entering V8; default-mode native state belongs only to |pid|. + int initialized; + int platform_mode; + int default_thread_started; + int init_result; + pid_t pid; int64_t idle_gc, max_memory, timeout; struct State *pst; // used by v8 thread VALUE procs; // array of js -> ruby callbacks @@ -194,6 +228,7 @@ static const rb_data_type_t snapshot_type = { static VALUE platform_init_error; static VALUE context_disposed_error; static VALUE pause_timeout_error; +static VALUE fork_error; static VALUE parse_error; static VALUE memory_error; static VALUE script_error; @@ -306,11 +341,37 @@ static void mini_racer_timespec_from_timeout(struct timespec *ts, double timeout } } +static int mini_racer_fork_error_p(int r) +{ + return r == MINI_RACER_FORK_CONTEXT_ERROR || + r == MINI_RACER_FORK_PLATFORM_ERROR || + r == MINI_RACER_FORK_BUSY_ERROR || + r == MINI_RACER_FORK_INIT_ERROR; +} + static const char *mini_racer_error_message(int r) { - if (r == ECANCELED) + switch (r) { + case ECANCELED: return "MiniRacer operation was interrupted or canceled"; - return strerror(r); + case MINI_RACER_FORK_CONTEXT_ERROR: + return "inherited default-platform context cannot be used after fork; configure :single_threaded in the parent before creating any Context or Snapshot"; + case MINI_RACER_FORK_PLATFORM_ERROR: + return "V8's default platform was initialized before fork and cannot be used in the child; configure :single_threaded in the parent before creating any Context or Snapshot"; + case MINI_RACER_FORK_BUSY_ERROR: + return "inherited single-threaded context was not quiescent at fork and cannot be recovered"; + case MINI_RACER_FORK_INIT_ERROR: + return "V8 initialization was in progress in another thread at fork and cannot continue in the child"; + default: + return strerror(r); + } +} + +static void mini_racer_raise_error(VALUE klass, const char *operation, int r) +{ + if (mini_racer_fork_error_p(r)) + klass = fork_error; + rb_raise(klass, "%s: %s", operation, mini_racer_error_message(r)); } static int mini_racer_operation_wait_unpaused(atomic_int *cancel, atomic_int *interrupted) @@ -1148,20 +1209,136 @@ void v8_reply(Context *c, const uint8_t *p, size_t n) pthread_mutex_unlock(&c->mtx); } -static void v8_once_init(void) +static MiniRacerV8InitState v8_init_state_pack(pid_t pid, int phase, int mode) +{ + return ((MiniRacerV8InitState)(uint64_t)pid << MINI_RACER_V8_STATE_PID_SHIFT) | + ((MiniRacerV8InitState)(mode != 0) << MINI_RACER_V8_STATE_MODE_SHIFT) | + (MiniRacerV8InitState)phase; +} + +static int v8_init_state_phase(MiniRacerV8InitState state) +{ + return (int)(state & MINI_RACER_V8_STATE_PHASE_MASK); +} + +static int v8_init_state_mode(MiniRacerV8InitState state) +{ + return (int)((state >> MINI_RACER_V8_STATE_MODE_SHIFT) & 1); +} + +static pid_t v8_init_state_pid(MiniRacerV8InitState state) +{ + return (pid_t)(state >> MINI_RACER_V8_STATE_PID_SHIFT); +} + +static int v8_effective_platform_mode(void) +{ + MiniRacerV8InitState state; + + state = atomic_load(&v8_init_state); + if (state) + return v8_init_state_mode(state); + return single_threaded ? MINI_RACER_PLATFORM_SINGLE_THREADED : + MINI_RACER_PLATFORM_DEFAULT; +} + +static int v8_init_preflight(int mode) +{ + MiniRacerV8InitState state; + pid_t pid; + + state = atomic_load(&v8_init_state); + if (!state) + return 0; + pid = getpid(); + if (v8_init_state_pid(state) == pid) + return v8_init_state_mode(state) == mode ? 0 : EINVAL; + if (v8_init_state_phase(state) != MINI_RACER_V8_COMPLETE) + return MINI_RACER_FORK_INIT_ERROR; + if (v8_init_state_mode(state) == MINI_RACER_PLATFORM_DEFAULT) + return MINI_RACER_FORK_PLATFORM_ERROR; + return 0; +} + +static int v8_init_claim(int mode) +{ + MiniRacerV8InitState state, desired; + pid_t pid; + int r; + + pid = getpid(); + for (;;) { + state = atomic_load(&v8_init_state); + if (state) + return v8_init_preflight(mode); + desired = v8_init_state_pack(pid, MINI_RACER_V8_STARTING, mode); + if (atomic_compare_exchange_weak(&v8_init_state, &state, desired)) + return 0; + if ((r = v8_init_preflight(mode))) + return r; + } +} + +static void v8_once_callback(void) +{ + MiniRacerV8InitState state; + + state = atomic_load(&v8_init_state); + v8_global_init(v8_init_state_mode(state)); +} + +static int v8_once_init(int mode) +{ + MiniRacerV8InitState state, complete; + pid_t pid; + int r; + + if ((r = v8_init_claim(mode))) + return r; + if ((r = pthread_once(&v8_once, v8_once_callback))) + return r; + + pid = getpid(); + for (;;) { + state = atomic_load(&v8_init_state); + if (v8_init_state_pid(state) != pid) { + if (v8_init_state_phase(state) == MINI_RACER_V8_COMPLETE && + v8_init_state_mode(state) == MINI_RACER_PLATFORM_SINGLE_THREADED) + return 0; + return v8_init_state_phase(state) == MINI_RACER_V8_COMPLETE ? + MINI_RACER_FORK_PLATFORM_ERROR : MINI_RACER_FORK_INIT_ERROR; + } + if (v8_init_state_phase(state) == MINI_RACER_V8_COMPLETE) + return 0; + complete = v8_init_state_pack(pid, MINI_RACER_V8_COMPLETE, + v8_init_state_mode(state)); + if (atomic_compare_exchange_weak(&v8_init_state, &state, complete)) + return 0; + } +} + +static int context_inherited_default(Context *c) { - static pthread_once_t once = PTHREAD_ONCE_INIT; - pthread_once(&once, v8_global_init); + return c->initialized && + c->platform_mode == MINI_RACER_PLATFORM_DEFAULT && + c->pid != getpid(); } static void *v8_thread_start(void *arg) { Context *c; + int r; c = arg; barrier_wait(&c->early_init); - v8_once_init(); - v8_thread_init(c, c->snapshot.buf, c->snapshot.len, c->max_memory, c->verbose_exceptions); + if ((r = v8_once_init(c->platform_mode))) { + c->init_result = r; + barrier_wait(&c->late_init); + return NULL; + } + v8_thread_init(c, c->snapshot.buf, c->snapshot.len, c->max_memory, + c->verbose_exceptions, + c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED); while (c->quit < 2) pthread_cond_wait(&c->cv, &c->mtx); context_destroy(c); @@ -1282,7 +1459,7 @@ static int single_threaded_recover_after_fork(Context *c) if (!c->single_threaded_thr_started || c->single_threaded_pid == pid) return 0; if (c->depth || c->req.len || c->res.len) - return EBUSY; + return MINI_RACER_FORK_BUSY_ERROR; if ((r = pthread_condattr_init(&cattr))) return r; @@ -1352,10 +1529,15 @@ static inline void *rendezvous_nogvl(void *arg) a->operation_entered = 1; } if (!a->started) { - if (single_threaded && (r = single_threaded_recover_after_fork(c))) { + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED && + (r = single_threaded_recover_after_fork(c))) { rendezvous_release(a); return (void *)(intptr_t)r; } + if (context_inherited_default(c)) { + rendezvous_release(a); + return (void *)(intptr_t)MINI_RACER_FORK_CONTEXT_ERROR; + } pthread_mutex_lock(&c->rr_mtx); if (atomic_load(&a->cancel)) { pthread_mutex_unlock(&c->rr_mtx); @@ -1385,7 +1567,7 @@ static inline void *rendezvous_nogvl(void *arg) assert(c->req.len == 0); assert(!c->res_ready); buf_move(a->req, &c->req); // v8 thread takes ownership of req - if (single_threaded) { + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED) { r = single_threaded_runner_start(c); if (r) { buf_move(&c->req, a->req); @@ -1447,6 +1629,8 @@ static void rendezvous_ubf(void *arg) if (!atomic_load(&a->active)) return; c = a->context; + if (context_inherited_default(c)) + return; pthread_cond_broadcast(&c->cv); } @@ -1466,6 +1650,8 @@ static void terminate_ubf(void *arg) atomic_store(&a->cancel, 1); mini_racer_pause_wakeup_all(); c = a->context; + if (context_inherited_default(c)) + return; if (c->pst) v8_terminate_execution(c->pst); pthread_cond_broadcast(&c->cv); @@ -1572,7 +1758,7 @@ static void rendezvous_no_des(Context *c, Buf *req, Buf *res) if ((int)(intptr_t)r == ECANCELED) rb_raise(context_disposed_error, "disposed context"); if (r) - rb_raise(runtime_error, "MiniRacer operation: %s", mini_racer_error_message((int)(intptr_t)r)); + mini_racer_raise_error(runtime_error, "MiniRacer operation", (int)(intptr_t)r); } // send request to & receive reply from v8 thread; takes ownership of |req| @@ -1747,7 +1933,8 @@ static void *context_free_do(void *arg) Context *c; c = arg; - if (single_threaded && single_threaded_recover_after_fork(c)) { + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED && + single_threaded_recover_after_fork(c)) { // The child forked while this inherited context was not idle. There is // no live runner thread to join and the inherited V8/pthread state is // not safe to tear down. A finalizer must not hang here; let the OS @@ -1755,7 +1942,8 @@ static void *context_free_do(void *arg) context_abandon(c); return NULL; } - if (single_threaded && c->single_threaded_thr_started && c->single_threaded_pid == getpid()) { + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED && + c->single_threaded_thr_started && c->single_threaded_pid == getpid()) { pthread_mutex_lock(&c->mtx); atomic_store(&c->quit, 2); pthread_cond_signal(&c->cv); @@ -1774,11 +1962,21 @@ static void context_free(void *arg) Context *c; c = arg; - if (single_threaded) { + if (!c->initialized) { + pthread_mutex_lock(&c->mtx); + context_destroy(c); + } else if (context_inherited_default(c)) { + // The context's V8 thread and the default platform's worker threads do + // not exist in the child. Do not touch inherited pthread or V8 state. + context_abandon(c); + } else if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED) { // Free synchronously. A detached cleanup thread can race normal Ruby // process shutdown and trip glibc malloc corruption checks while V8 is // tearing down single-threaded contexts. context_free_do(c); + } else if (!c->default_thread_started) { + pthread_mutex_lock(&c->mtx); + context_destroy(c); } else { pthread_mutex_lock(&c->mtx); c->quit = 2; // 2 = v8 thread frees @@ -1863,7 +2061,11 @@ static void *context_dispose_do(void *arg) ret = NULL; if ((r = mini_racer_operation_enter_nogvl(&a->cancel, NULL, &a->counted))) return (void *)(intptr_t)r; - if (single_threaded) { + if (context_inherited_default(c)) { + atomic_store(&c->quit, 1); + goto out; + } + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED) { if ((r = single_threaded_recover_after_fork(c))) { ret = (void *)(intptr_t)r; goto out; @@ -1888,7 +2090,7 @@ static void *context_dispose_do(void *arg) v8_terminate_execution(c->pst); pthread_cond_broadcast(&c->cv); } - if (single_threaded) { + if (c->platform_mode == MINI_RACER_PLATFORM_SINGLE_THREADED) { pthread_mutex_lock(&c->mtx); while ((c->req.len || c->res.len) && !atomic_load(&a->cancel)) pthread_cond_wait(&c->cv, &c->mtx); @@ -1931,12 +2133,16 @@ static VALUE context_dispose(VALUE self) void *r; TypedData_Get_Struct(self, Context, &context_type, c); + if (context_inherited_default(c)) { + atomic_store(&c->quit, 1); + return Qnil; + } a.context = c; a.counted = 0; atomic_init(&a.cancel, 0); r = rb_thread_call_without_gvl(context_dispose_do, &a, terminate_ubf, &a); if (r) - rb_raise(runtime_error, "context dispose: %s", mini_racer_error_message((int)(intptr_t)r)); + mini_racer_raise_error(runtime_error, "context dispose", (int)(intptr_t)r); return Qnil; } @@ -1949,6 +2155,8 @@ static VALUE context_stop(VALUE self) TypedData_Get_Struct(self, Context, &context_type, c); if (atomic_load(&c->quit)) rb_raise(context_disposed_error, "disposed context"); + if (context_inherited_default(c)) + mini_racer_raise_error(runtime_error, "context stop", MINI_RACER_FORK_CONTEXT_ERROR); v8_terminate_execution(c->pst); return Qnil; } @@ -2158,10 +2366,14 @@ static int platform_set_flag1(VALUE k, VALUE v) *r = '\0'; } p = buf; + // Avoid inherited flag synchronization entirely once initialization has + // started. v8_init_state is published before v8_get_flags takes flags_mtx. + if (atomic_load(&v8_init_state)) + return 0; pthread_mutex_lock(&flags_mtx); if (!flags.buf) buf_init(&flags); - ok = (*flags.buf != 1); + ok = (!atomic_load(&v8_init_state) && *flags.buf != 1); if (ok) { buf_put(&flags, p, 1+strlen(p)); // include trailing \0 // strip dashes and underscores to reduce the number of variant @@ -2185,6 +2397,22 @@ static int platform_set_flag1(VALUE k, VALUE v) return ok; } +static VALUE platform_fork_safety_status(VALUE klass) +{ + MiniRacerV8InitState state; + + (void)&klass; + state = atomic_load(&v8_init_state); + if (!state) + return ID2SYM(rb_intern(single_threaded ? "single_threaded" : "default")); + if (v8_init_state_pid(state) != getpid() && + v8_init_state_phase(state) != MINI_RACER_V8_COMPLETE) + return ID2SYM(rb_intern("inherited_initialization")); + if (v8_init_state_mode(state) == MINI_RACER_PLATFORM_SINGLE_THREADED) + return ID2SYM(rb_intern("single_threaded")); + return ID2SYM(rb_intern("default_initialized")); +} + static VALUE platform_set_flags(int argc, VALUE *argv, VALUE klass) { VALUE args, kwargs, k, v; @@ -2368,7 +2596,7 @@ void v8_get_flags(char **p, size_t *n) buf_init(&flags); buf_putc(&flags, 1); // marker to indicate it's been cleared pthread_mutex_unlock(&flags_mtx); - if (single_threaded) + if (v8_effective_platform_mode() == MINI_RACER_PLATFORM_SINGLE_THREADED) rb_thread_lock_native_thread(); } @@ -2385,7 +2613,7 @@ static VALUE context_initialize_do(VALUE arg) const char *cause; pthread_t thr; Context *c; - int r; + int mode, r; a = (struct context_initialize_args *)arg; c = a->context; @@ -2393,16 +2621,27 @@ static VALUE context_initialize_do(VALUE arg) cause = "MiniRacer operation"; if ((r = mini_racer_operation_enter_gvl(&a->counted))) goto fail; - if (single_threaded) { - v8_once_init(); - c->pst = v8_thread_init(c, c->snapshot.buf, c->snapshot.len, c->max_memory, c->verbose_exceptions); + + mode = v8_effective_platform_mode(); + c->pid = getpid(); + c->platform_mode = mode; + cause = "V8 initialization"; + if ((r = v8_init_preflight(mode))) + goto fail; + + if (mode == MINI_RACER_PLATFORM_SINGLE_THREADED) { + if ((r = v8_once_init(mode))) + goto fail; + c->pst = v8_thread_init(c, c->snapshot.buf, c->snapshot.len, + c->max_memory, c->verbose_exceptions, 1); + c->initialized = 1; } else { cause = "pthread_attr_init"; if ((r = pthread_attr_init(&attr))) goto fail; pthread_attr_setstacksize(&attr, 2<<20); // 2 MiB - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - // v8 thread takes ownership of |c| + // Keep the thread joinable until initialization succeeds so a defensive + // worker-side fork rejection can be joined before the Context is freed. cause = "pthread_create"; r = pthread_create(&thr, &attr, v8_thread_start, c); pthread_attr_destroy(&attr); @@ -2410,10 +2649,20 @@ static VALUE context_initialize_do(VALUE arg) goto fail; barrier_wait(&c->early_init); barrier_wait(&c->late_init); + if ((r = c->init_result)) { + pthread_join(thr, NULL); + goto fail; + } + pthread_detach(thr); + c->default_thread_started = 1; + c->initialized = 1; } return Qnil; fail: - rb_raise(runtime_error, "Context.initialize: %s: %s", cause, mini_racer_error_message(r)); + if (mini_racer_fork_error_p(r)) + mini_racer_raise_error(runtime_error, "Context.initialize", r); + rb_raise(runtime_error, "Context.initialize: %s: %s", cause, + mini_racer_error_message(r)); return Qnil; // pacify compiler } @@ -2612,6 +2861,9 @@ void Init_mini_racer_extension(void) { VALUE c, m; + if (!atomic_is_lock_free(&v8_init_state)) + rb_raise(rb_eLoadError, + "mini_racer requires lock-free 64-bit atomics for fork safety"); m = rb_define_module("MiniRacer"); mini_racer_pause_state_init(0); c = rb_define_class_under(m, "Error", rb_eStandardError); @@ -2619,6 +2871,7 @@ void Init_mini_racer_extension(void) platform_init_error = rb_define_class_under(m, "PlatformAlreadyInitialized", c); context_disposed_error = rb_define_class_under(m, "ContextDisposedError", c); pause_timeout_error = rb_define_class_under(m, "PauseTimeoutError", c); + fork_error = rb_define_class_under(m, "ForkError", c); rb_define_singleton_method(m, "pause", mini_racer_pause, -1); rb_define_singleton_method(m, "resume", mini_racer_resume, 0); @@ -2659,6 +2912,8 @@ void Init_mini_racer_extension(void) c = rb_define_class_under(m, "Platform", rb_cObject); rb_define_singleton_method(c, "set_flags!", platform_set_flags, -1); + rb_define_private_method(rb_singleton_class(c), "_fork_safety_status", + platform_fork_safety_status, 0); date_time_class = Qnil; // lazy init binary_class = Qnil; // lazy init diff --git a/ext/mini_racer_extension/mini_racer_v8.cc b/ext/mini_racer_extension/mini_racer_v8.cc index 3f36350..a80e019 100644 --- a/ext/mini_racer_extension/mini_racer_v8.cc +++ b/ext/mini_racer_extension/mini_racer_v8.cc @@ -401,7 +401,7 @@ v8::Local to_error(State& st, v8::TryCatch *try_catch, int cause) return string_from_bytes(st.isolate, buf); } -extern "C" void v8_global_init(void) +extern "C" void v8_global_init(int platform_single_threaded) { char *p; size_t n; @@ -414,7 +414,7 @@ extern "C" void v8_global_init(void) free(p); } v8::V8::InitializeICU(); - if (single_threaded) { + if (platform_single_threaded) { platform = v8::platform::NewSingleThreadedDefaultPlatform().release(); } else { platform = v8::platform::NewDefaultPlatform().release(); @@ -437,8 +437,9 @@ void v8_gc_callback(v8::Isolate*, v8::GCType, v8::GCCallbackFlags, void *data) } extern "C" State *v8_thread_init(Context *c, const uint8_t *snapshot_buf, - size_t snapshot_len, int64_t max_memory, - int verbose_exceptions) + size_t snapshot_len, int64_t max_memory, + int verbose_exceptions, + int platform_single_threaded) { State *pst = new State{}; State& st = *pst; @@ -485,7 +486,7 @@ extern "C" State *v8_thread_init(Context *c, const uint8_t *snapshot_buf, st.safe_context->UseDefaultSecurityToken(); st.safe_context_function = v8::Local::Cast(function_v); } - if (single_threaded) { + if (platform_single_threaded) { st.persistent_safe_context_function.Reset(st.isolate, st.safe_context_function); st.persistent_safe_context.Reset(st.isolate, st.safe_context); st.persistent_context.Reset(st.isolate, st.context); diff --git a/ext/mini_racer_extension/mini_racer_v8.h b/ext/mini_racer_extension/mini_racer_v8.h index 947404d..aebbd28 100644 --- a/ext/mini_racer_extension/mini_racer_v8.h +++ b/ext/mini_racer_extension/mini_racer_v8.h @@ -25,7 +25,6 @@ struct Context; struct State; // defined in mini_racer_extension.c -extern int single_threaded; void v8_get_flags(char **p, size_t *n); void v8_thread_main(struct Context *c, struct State *pst); void v8_dispatch(struct Context *c); @@ -33,10 +32,11 @@ void v8_reply(struct Context *c, const uint8_t *p, size_t n); void v8_roundtrip(struct Context *c, const uint8_t **p, size_t *n); // defined in mini_racer_v8.cc -void v8_global_init(void); +void v8_global_init(int platform_single_threaded); struct State *v8_thread_init(struct Context *c, const uint8_t *snapshot_buf, size_t snapshot_len, int64_t max_memory, - int verbose_exceptions); // calls v8_thread_main + int verbose_exceptions, + int platform_single_threaded); // calls v8_thread_main void v8_attach(struct State *pst, const uint8_t *p, size_t n); void v8_call(struct State *pst, const uint8_t *p, size_t n); void v8_call_await(struct State *pst, const uint8_t *p, size_t n); diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 3d2c218..3527320 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -49,6 +49,8 @@ class PlatformAlreadyInitialized < Error end class PauseTimeoutError < Error end + class ForkError < Error + end class EvalError < Error end @@ -123,6 +125,7 @@ def install_fork_hooks!(timeout: 5.0) end end + warn_about_default_fork_platform @fork_hook_timeout = timeout unless @fork_hooks_installed Process.singleton_class.prepend(ForkHooks) @@ -130,6 +133,33 @@ def install_fork_hooks!(timeout: 5.0) end true end + + private + + def warn_about_default_fork_platform + return unless Platform.respond_to?(:_fork_safety_status, true) + + status = Platform.__send__(:_fork_safety_status) + return if status == :single_threaded + return if @fork_hook_warning_pid == Process.pid + + detail = + case status + when :default_initialized + "V8's default platform is already initialized in this process. " + when :inherited_initialization + "The process inherited V8 initialization that was in progress at fork. " + else + "The default V8 platform is currently configured. " + end + warn "mini_racer: #{detail}" \ + "Fork hooks only quiesce MiniRacer operations; they cannot make an " \ + "initialized default V8 worker pool usable in a child. If forked " \ + "children use MiniRacer, configure " \ + "MiniRacer::Platform.set_flags!(:single_threaded) before creating " \ + "any Context or Snapshot." + @fork_hook_warning_pid = Process.pid + end end class ScriptError < EvalError diff --git a/test/fork_safety_test.rb b/test/fork_safety_test.rb new file mode 100644 index 0000000..bcbd35b --- /dev/null +++ b/test/fork_safety_test.rb @@ -0,0 +1,330 @@ +# frozen_string_literal: true + +require "open3" +require "rbconfig" +require "tempfile" +require "test_helper" + +class MiniRacerForkSafetyTest < Minitest::Test + def assert_fork_script(script) + skip "fork safety tests are only for CRuby" unless RUBY_ENGINE == "ruby" + skip "fork is not available" unless Process.respond_to?(:fork) + + file = Tempfile.new(%w[mini_racer_fork_safety .rb]) + file.write(<<~RUBY) + $LOAD_PATH.unshift #{File.expand_path("../lib", __dir__).inspect} + require "mini_racer" + + Thread.new do + sleep 15 + warn "fork safety script timed out" + exit! 98 + end + + def child_watchdog + Thread.new do + sleep 3 + warn "child timed out" + exit! 99 + end + end + + #{script} + RUBY + file.close + + stdout, stderr, status = Open3.capture3(RbConfig.ruby, file.path) + assert status.success?, <<~MSG + fork safety script failed with status #{status.exitstatus} + stdout: + #{stdout} + stderr: + #{stderr} + MSG + [stdout, stderr] + ensure + file&.unlink + end + + def test_fork_error_is_a_mini_racer_error + assert_operator MiniRacer::ForkError, :<, MiniRacer::Error + end + + def test_inherited_default_context_fails_promptly + assert_fork_script <<~'RUBY' + context = MiniRacer::Context.new + raise "bad parent eval" unless context.eval("var answer = 42; answer") == 42 + + pid = fork do + child_watchdog + begin + context.eval("answer") + warn "inherited eval unexpectedly succeeded" + exit! 1 + rescue MiniRacer::ForkError => e + unless e.message.include?("inherited default-platform context") && + e.message.include?(":single_threaded") + warn "bad fork error: #{e.message}" + exit! 2 + end + exit! 0 + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + end + + def test_new_default_context_fails_after_parent_initialization + assert_fork_script <<~'RUBY' + context = MiniRacer::Context.new + context.eval("1 + 1") + + pid = fork do + child_watchdog + begin + MiniRacer::Context.new + warn "child context unexpectedly initialized" + exit! 1 + rescue MiniRacer::ForkError => e + unless e.message.include?("default platform was initialized before fork") && + e.message.include?(":single_threaded") + warn "bad platform fork error: #{e.message}" + exit! 2 + end + exit! 0 + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("20 + 22") == 42 + RUBY + end + + def test_snapshot_initialization_also_guards_the_default_platform + assert_fork_script <<~'RUBY' + MiniRacer::Snapshot.new("var snap = 42") + + pid = fork do + child_watchdog + begin + MiniRacer::Snapshot.new + warn "child snapshot unexpectedly initialized" + exit! 1 + rescue MiniRacer::ForkError => e + exit!(e.message.include?("default platform was initialized before fork") ? 0 : 2) + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + RUBY + end + + def test_inherited_default_context_finalizer_does_not_hang + assert_fork_script <<~'RUBY' + require "weakref" + + context = MiniRacer::Context.new + context.eval("1 + 1") + weak_context = WeakRef.new(context) + + pid = fork do + child_watchdog + context = nil + 5.times do + GC.start(full_mark: true, immediate_sweep: true) + GC.compact if GC.respond_to?(:compact) + break unless weak_context.weakref_alive? + end + raise "inherited context was not finalized" if weak_context.weakref_alive? + # Intentionally fall off the end: normal child shutdown must also avoid + # touching the inherited default platform or its missing threads. + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("20 + 22") == 42 + RUBY + end + + def test_inherited_default_context_dispose_and_stop_are_safe + assert_fork_script <<~'RUBY' + context = MiniRacer::Context.new + context.eval("var answer = 42") + + dispose_pid = fork do + child_watchdog + context.dispose + begin + context.eval("answer") + exit! 1 + rescue MiniRacer::ContextDisposedError + exit! 0 + end + end + _, dispose_status = Process.wait2(dispose_pid) + raise "dispose child failed with #{dispose_status.inspect}" unless dispose_status.success? + + stop_pid = fork do + child_watchdog + begin + context.stop + exit! 1 + rescue MiniRacer::ForkError => e + exit!(e.message.include?("inherited default-platform context") ? 0 : 2) + end + end + _, stop_status = Process.wait2(stop_pid) + raise "stop child failed with #{stop_status.inspect}" unless stop_status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + end + + def test_default_platform_can_initialize_for_the_first_time_in_child + assert_fork_script <<~'RUBY' + pid = fork do + child_watchdog + context = MiniRacer::Context.new + exit!(context.eval("6 * 7") == 42 ? 0 : 1) + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + RUBY + end + + def test_raw_fork_racing_first_default_initialization_never_hangs + assert_fork_script <<~'RUBY' + ready_r, ready_w = IO.pipe + initializer = Thread.new do + ready_w.write("x") + ready_w.flush + context = MiniRacer::Context.new + raise "bad initializer result" unless context.eval("6 * 7") == 42 + end + ready_r.read(1) + + pid = fork do + child_watchdog + begin + context = MiniRacer::Context.new + exit!(context.eval("6 * 7") == 42 ? 0 : 1) + rescue MiniRacer::ForkError => e + valid = e.message.include?("initialization was in progress") || + e.message.include?("default platform was initialized before fork") + exit!(valid ? 0 : 2) + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent initializer did not finish" unless initializer.join(10) + RUBY + end + + def test_child_can_avoid_mini_racer_after_parent_default_initialization + assert_fork_script <<~'RUBY' + context = MiniRacer::Context.new + context.eval("var answer = 42") + + pid = fork do + child_watchdog + exit! 0 + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + end + + def test_child_cannot_switch_an_inherited_default_platform + assert_fork_script <<~'RUBY' + MiniRacer::Context.new + + pid = fork do + child_watchdog + begin + MiniRacer::Platform.set_flags!(:single_threaded) + exit! 1 + rescue MiniRacer::PlatformAlreadyInitialized + exit! 0 + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + RUBY + end + + def test_fork_hooks_do_not_make_initialized_default_platform_reusable + _stdout, stderr = + assert_fork_script <<~'RUBY' + MiniRacer.install_fork_hooks!(timeout: 1) + context = MiniRacer::Context.new + context.eval("var answer = 42") + + pid = fork do + child_watchdog + begin + MiniRacer::Context.new + exit! 1 + rescue MiniRacer::ForkError + exit! 0 + end + end + _, status = Process.wait2(pid) + raise "child failed with #{status.inspect}" unless status.success? + raise "parent context broke" unless context.eval("answer") == 42 + RUBY + + assert_includes stderr, "Fork hooks only quiesce MiniRacer operations" + end + + def test_default_platform_fork_hook_rejects_fork_from_callback + assert_fork_script <<~'RUBY' + MiniRacer.install_fork_hooks!(timeout: 1) + context = MiniRacer::Context.new + context.attach("try_fork", proc do + begin + fork { exit! 88 } + "forked" + rescue MiniRacer::RuntimeError => e + raise unless e.message.include?("cannot pause") + "rejected" + end + end) + + raise "fork was not rejected" unless context.eval("try_fork()") == "rejected" + raise "context should still be usable" unless context.eval("1 + 1") == 2 + RUBY + end + + def test_default_platform_hook_warning_is_emitted_once + _stdout, stderr = + assert_fork_script <<~'RUBY' + MiniRacer.install_fork_hooks!(timeout: 1) + MiniRacer.install_fork_hooks!(timeout: 2) + raise "timeout was not updated" unless MiniRacer.fork_hook_timeout == 2.0 + RUBY + + assert_equal 1, stderr.scan("Fork hooks only quiesce MiniRacer operations").length + assert_includes stderr, ":single_threaded" + end + + def test_initialized_default_platform_gets_stronger_hook_warning + _stdout, stderr = + assert_fork_script <<~'RUBY' + MiniRacer::Context.new + MiniRacer.install_fork_hooks!(timeout: 1) + RUBY + + assert_includes stderr, "default platform is already initialized" + end + + def test_single_threaded_configuration_does_not_warn + _stdout, stderr = + assert_fork_script <<~'RUBY' + MiniRacer::Platform.set_flags!(:single_threaded) + MiniRacer.install_fork_hooks!(timeout: 1) + RUBY + + refute_includes stderr, "Fork hooks only quiesce MiniRacer operations" + end +end diff --git a/test/single_threaded_test.rb b/test/single_threaded_test.rb index 9598b3b..1108f26 100644 --- a/test/single_threaded_test.rb +++ b/test/single_threaded_test.rb @@ -18,6 +18,12 @@ def assert_single_threaded_script(script) MiniRacer::Platform.set_flags!(:single_threaded) + Thread.new do + sleep 15 + warn "single-threaded script timed out" + exit! 98 + end + #{script} RUBY file.close @@ -365,6 +371,47 @@ def test_fork_child_gc_after_non_idle_inherited_context RUBY end + def test_non_idle_inherited_context_raises_fork_error + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + started_r, started_w = IO.pipe + release_r, release_w = IO.pipe + context = MiniRacer::Context.new + context.attach("block", proc do + started_w.write("x") + started_w.flush + release_r.read(1) + 42 + end) + + worker = Thread.new { context.eval("block()") } + started_r.read(1) + + pid = fork do + Thread.new do + sleep 3 + warn "child timed out" + exit! 99 + end + + begin + context.eval("1 + 1") + exit! 1 + rescue MiniRacer::ForkError => e + exit!(e.message.include?("was not quiescent at fork") ? 0 : 2) + end + end + _, status = Process.wait2(pid) + raise "child failed with status #{status.inspect}" unless status.success? + + release_w.write("x") + release_w.flush + raise "parent worker did not finish" unless worker.join(3) + raise "parent context broke" unless context.eval("1 + 1") == 2 + RUBY + end + def test_fork_after_low_memory_notification assert_single_threaded_script <<~'RUBY' exit 0 unless Process.respond_to?(:fork) @@ -389,6 +436,34 @@ def test_fork_after_low_memory_notification RUBY end + def test_fork_child_can_create_new_context_after_parent_initialization + assert_single_threaded_script <<~'RUBY' + exit 0 unless Process.respond_to?(:fork) + + parent_context = MiniRacer::Context.new + parent_context.eval("var answer = 42") + + pid = fork do + Thread.new do + sleep 3 + warn "child timed out" + exit! 99 + end + + begin + child_context = MiniRacer::Context.new + exit!(child_context.eval("6 * 7") == 42 ? 0 : 1) + rescue MiniRacer::ForkError => e + warn "single-threaded child was incorrectly rejected: #{e.message}" + exit! 2 + end + end + _, status = Process.wait2(pid) + raise "child failed with status #{status.inspect}" unless status.success? + raise "parent context broke" unless parent_context.eval("answer") == 42 + RUBY + end + def test_fork_hook_pauses_and_recovers_child assert_single_threaded_script <<~'RUBY' exit 0 unless Process.respond_to?(:fork) From ae38066236ad38a0d35ebe086dd8549301b76e02 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 13 Aug 2026 13:31:43 +1000 Subject: [PATCH 4/4] lint --- lib/mini_racer.rb | 10 +++++----- test/fork_safety_test.rb | 17 ++++++++--------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 3527320..863e1b3 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -153,11 +153,11 @@ def warn_about_default_fork_platform "The default V8 platform is currently configured. " end warn "mini_racer: #{detail}" \ - "Fork hooks only quiesce MiniRacer operations; they cannot make an " \ - "initialized default V8 worker pool usable in a child. If forked " \ - "children use MiniRacer, configure " \ - "MiniRacer::Platform.set_flags!(:single_threaded) before creating " \ - "any Context or Snapshot." + "Fork hooks only quiesce MiniRacer operations; they cannot make an " \ + "initialized default V8 worker pool usable in a child. If forked " \ + "children use MiniRacer, configure " \ + "MiniRacer::Platform.set_flags!(:single_threaded) before creating " \ + "any Context or Snapshot." @fork_hook_warning_pid = Process.pid end end diff --git a/test/fork_safety_test.rb b/test/fork_safety_test.rb index bcbd35b..c2ec25b 100644 --- a/test/fork_safety_test.rb +++ b/test/fork_safety_test.rb @@ -254,8 +254,7 @@ def test_child_cannot_switch_an_inherited_default_platform end def test_fork_hooks_do_not_make_initialized_default_platform_reusable - _stdout, stderr = - assert_fork_script <<~'RUBY' + _stdout, stderr = assert_fork_script <<~'RUBY' MiniRacer.install_fork_hooks!(timeout: 1) context = MiniRacer::Context.new context.eval("var answer = 42") @@ -297,20 +296,21 @@ def test_default_platform_fork_hook_rejects_fork_from_callback end def test_default_platform_hook_warning_is_emitted_once - _stdout, stderr = - assert_fork_script <<~'RUBY' + _stdout, stderr = assert_fork_script <<~'RUBY' MiniRacer.install_fork_hooks!(timeout: 1) MiniRacer.install_fork_hooks!(timeout: 2) raise "timeout was not updated" unless MiniRacer.fork_hook_timeout == 2.0 RUBY - assert_equal 1, stderr.scan("Fork hooks only quiesce MiniRacer operations").length + assert_equal 1, + stderr.scan( + "Fork hooks only quiesce MiniRacer operations" + ).length assert_includes stderr, ":single_threaded" end def test_initialized_default_platform_gets_stronger_hook_warning - _stdout, stderr = - assert_fork_script <<~'RUBY' + _stdout, stderr = assert_fork_script <<~'RUBY' MiniRacer::Context.new MiniRacer.install_fork_hooks!(timeout: 1) RUBY @@ -319,8 +319,7 @@ def test_initialized_default_platform_gets_stronger_hook_warning end def test_single_threaded_configuration_does_not_warn - _stdout, stderr = - assert_fork_script <<~'RUBY' + _stdout, stderr = assert_fork_script <<~'RUBY' MiniRacer::Platform.set_flags!(:single_threaded) MiniRacer.install_fork_hooks!(timeout: 1) RUBY