From 25ecf404992ed67bcd3f42fbbd5f2af20c194a74 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 30 Oct 2026 18:43:53 +0100 Subject: [PATCH 1/5] refactor(core): initialize instrument-hooks on first use Importing the native module used to create the instrument-hooks instance, which opens the runner FIFO and writes environment data. Create it on the first hook call instead, so loading the module has no instrumentation side effects. --- .../native_core/instruments/hooks_wrapper.cc | 33 ++++++++++--------- .../native_core/instruments/hooks_wrapper.h | 5 --- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/core/src/native_core/instruments/hooks_wrapper.cc b/packages/core/src/native_core/instruments/hooks_wrapper.cc index c616e61a..2133886c 100644 --- a/packages/core/src/native_core/instruments/hooks_wrapper.cc +++ b/packages/core/src/native_core/instruments/hooks_wrapper.cc @@ -6,32 +6,35 @@ namespace codspeed_native { namespace instruments { namespace hooks_wrapper { -static InstrumentHooks *hooks = nullptr; +namespace { -void InitializeGlobal() { - if (!hooks) { - hooks = instrument_hooks_init(); - } +// Created on first use: init opens the runner FIFO, so importing the module must +// not trigger it. +InstrumentHooks *GetHooks() { + static InstrumentHooks *hooks = instrument_hooks_init(); + return hooks; } +} // namespace + Napi::Boolean IsInstrumented(const Napi::CallbackInfo &info) { Napi::Env env = info.Env(); - bool instrumented = instrument_hooks_is_instrumented(hooks); + bool instrumented = instrument_hooks_is_instrumented(GetHooks()); return Napi::Boolean::New(env, instrumented); } Napi::Number StartBenchmark(const Napi::CallbackInfo &info) { Napi::Env env = info.Env(); - uint8_t result = instrument_hooks_start_benchmark(hooks); + uint8_t result = instrument_hooks_start_benchmark(GetHooks()); return Napi::Number::New(env, result); } Napi::Number StopBenchmark(const Napi::CallbackInfo &info) { Napi::Env env = info.Env(); - uint8_t result = instrument_hooks_stop_benchmark(hooks); + uint8_t result = instrument_hooks_stop_benchmark(GetHooks()); return Napi::Number::New(env, result); } @@ -54,7 +57,7 @@ Napi::Number SetExecutedBenchmark(const Napi::CallbackInfo &info) { std::string uri = info[1].As().Utf8Value(); uint8_t result = - instrument_hooks_set_executed_benchmark(hooks, pid, uri.c_str()); + instrument_hooks_set_executed_benchmark(GetHooks(), pid, uri.c_str()); return Napi::Number::New(env, result); } @@ -76,8 +79,8 @@ Napi::Number SetIntegration(const Napi::CallbackInfo &info) { std::string name = info[0].As().Utf8Value(); std::string version = info[1].As().Utf8Value(); - uint8_t result = - instrument_hooks_set_integration(hooks, name.c_str(), version.c_str()); + uint8_t result = instrument_hooks_set_integration( + GetHooks(), name.c_str(), version.c_str()); return Napi::Number::New(env, result); } @@ -104,7 +107,7 @@ Napi::Number SetEnvironment(const Napi::CallbackInfo &info) { std::string value = info[2].As().Utf8Value(); uint8_t result = instrument_hooks_set_environment( - hooks, section_name.c_str(), key.c_str(), value.c_str()); + GetHooks(), section_name.c_str(), key.c_str(), value.c_str()); return Napi::Number::New(env, result); } @@ -125,7 +128,7 @@ Napi::Number WriteEnvironment(const Napi::CallbackInfo &info) { int32_t pid = info[0].As().Int32Value(); - uint8_t result = instrument_hooks_write_environment(hooks, pid); + uint8_t result = instrument_hooks_write_environment(GetHooks(), pid); return Napi::Number::New(env, result); } @@ -193,7 +196,7 @@ Napi::Number AddMarker(const Napi::CallbackInfo &info) { } uint8_t result = - instrument_hooks_add_marker(hooks, pid, marker_type, timestamp); + instrument_hooks_add_marker(GetHooks(), pid, marker_type, timestamp); return Napi::Number::New(env, result); } @@ -221,8 +224,6 @@ Napi::Value __attribute__ ((noinline)) __codspeed_root_frame__(const Napi::Callb Napi::Object Initialize(Napi::Env env, Napi::Object exports) { Napi::Object instrumentHooksObj = Napi::Object::New(env); - InitializeGlobal(); - instrumentHooksObj.Set(Napi::String::New(env, "isInstrumented"), Napi::Function::New(env, IsInstrumented)); instrumentHooksObj.Set(Napi::String::New(env, "startBenchmark"), diff --git a/packages/core/src/native_core/instruments/hooks_wrapper.h b/packages/core/src/native_core/instruments/hooks_wrapper.h index d5c92daf..dd62e9a8 100644 --- a/packages/core/src/native_core/instruments/hooks_wrapper.h +++ b/packages/core/src/native_core/instruments/hooks_wrapper.h @@ -7,11 +7,6 @@ namespace codspeed_native { namespace instruments { namespace hooks_wrapper { -// Global initialization -// WARNING: InitializeGlobal() must be called before using any other functions -// All API functions assume the global instance is already initialized -void InitializeGlobal(); - Napi::Boolean IsInstrumented(const Napi::CallbackInfo &info); Napi::Number StartBenchmark(const Napi::CallbackInfo &info); Napi::Number StopBenchmark(const Napi::CallbackInfo &info); From 8ef4d2e472cbdb5248206b82a6a423ac14eed848 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 30 Oct 2026 18:43:54 +0100 Subject: [PATCH 2/5] fix(core): restart node with the required V8 flags Launchers that start node by absolute path, such as Bazel's rules_js, never run the runner's node wrapper on PATH. tinybench and benchmark.js then measured without the flags getV8Flags() requires: --no-opt, --predictable, the seed flags, and the GC flags. ensureV8Flags() now replaces the process before any benchmark or instrument-hooks state exists, inserting only the missing flags before the current execArgv so user flags keep precedence, as with the runner's wrapper. The restart reuses the running executable: /proc/self/exe on Linux because rules_js replaces process.execPath with its shell shim, which would re-add --require. A native execv binding keeps the PID, environment and open descriptors, so cancellation and the runner FIFO are unaffected; it clears FD_CLOEXEC on stdio like process.execve, otherwise the restarted process writes to /dev/null. The PID marker stops a restart loop when the launcher strips the flags again. Worker threads, IPC children, unsupported platforms, unbound addons and a failed exec keep the previous warning behavior instead of aborting the benchmark. Closes COD-4038 --- packages/benchmark.js-plugin/src/index.ts | 3 + packages/core/src/index.ts | 2 +- packages/core/src/introspection.ts | 76 +++++++++- packages/core/src/native_core/index.ts | 6 + packages/core/src/native_core/native_core.cc | 132 ++++++++++++++++++ .../core/tests/ensureV8Flags.integ.test.ts | 109 +++++++++++++++ packages/tinybench-plugin/src/index.ts | 3 + 7 files changed, 328 insertions(+), 3 deletions(-) create mode 100644 packages/core/tests/ensureV8Flags.integ.test.ts diff --git a/packages/benchmark.js-plugin/src/index.ts b/packages/benchmark.js-plugin/src/index.ts index fa69a35a..dcbaf158 100644 --- a/packages/benchmark.js-plugin/src/index.ts +++ b/packages/benchmark.js-plugin/src/index.ts @@ -1,4 +1,5 @@ import { + ensureV8Flags, getCallingFile, InstrumentHooks, mongoMeasurement, @@ -19,6 +20,8 @@ import { CodSpeedBenchmark } from "./types"; declare const __VERSION__: string; tryIntrospect(); +// Must run before any benchmark state exists: it may replace this process. +ensureV8Flags(); interface WithCodSpeedBenchmark extends Omit< Benchmark, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8dc74641..e281405e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -52,7 +52,7 @@ export type { SetupInstrumentsRequestBody, SetupInstrumentsResponse, } from "./generated/openapi"; -export { getV8Flags, tryIntrospect } from "./introspection"; +export { ensureV8Flags, getV8Flags, tryIntrospect } from "./introspection"; export { getUnsupportedNodeVersionWarning, SUPPORTED_NODE_MAJORS, diff --git a/packages/core/src/introspection.ts b/packages/core/src/introspection.ts index 8906c1d9..982ab2c8 100644 --- a/packages/core/src/introspection.ts +++ b/packages/core/src/introspection.ts @@ -1,11 +1,14 @@ -import { writeFileSync } from "fs"; +import { readlinkSync, writeFileSync } from "fs"; import path from "path"; +import { isMainThread } from "worker_threads"; -import { getInstrumentMode } from "./runnerMode"; +import native_core from "./native_core"; +import { getCodspeedRunnerMode, getInstrumentMode } from "./runnerMode"; const CUSTOM_INTROSPECTION_EXIT_CODE = 0; const V8_LOG_FILENAME_PATTERN = "codspeed-v8-%p.log"; +const RESTARTED_PID_ENV = "__CODSPEED_NODE_CORE_RESTARTED_PID__"; // Fixed seeds so Math.random() and hash table layout match on every run const getSeedFlags = (nodeVersionMajor: number) => { @@ -91,6 +94,75 @@ export const tryIntrospect = () => { } }; +/** + * Restarts the process with the V8 flags the runner mode requires when the + * launcher did not pass them (e.g. Bazel executes node by absolute path, so the + * runner's `node` wrapper never runs). Must be called before instrument-hooks + * is used: the restart replaces this process. + * + * Cases that cannot restart safely, including a failed exec, run without the + * flags; `checkV8Flags` warns once instrument-hooks is set up. + */ +export const ensureV8Flags = () => { + if (getCodspeedRunnerMode() === "disabled" || !native_core.isBound) { + return; + } + + const missingFlags = getV8Flags().filter( + (flag) => !process.execArgv.includes(flag), + ); + if (missingFlags.length === 0) { + return; + } + + // Worker threads and IPC children cannot replace their process, and an IPC + // channel would be lost. + if (!isMainThread || process.send !== undefined) { + return; + } + + // The restart did not help (its launcher stripped the flags again): do not + // restart in a loop. + if (process.env[RESTARTED_PID_ENV] === String(process.pid)) { + return; + } + + const getExecutable = () => { + switch (process.platform) { + case "linux": + // rules_js replaces process.execPath with its shell shim. + return readlinkSync("/proc/self/exe"); + case "darwin": + return process.execPath; + default: + return undefined; + } + }; + + try { + const executable = getExecutable(); + if (executable === undefined) { + return; + } + + process.env[RESTARTED_PID_ENV] = String(process.pid); + // The runner's wrapper passes its flags before the user's, so user flags win. + native_core.restart(executable, [ + executable, + ...missingFlags, + ...process.execArgv, + ...process.argv.slice(1), + ]); + } catch (error) { + // No /proc, or exec failed: the current image is still usable, so measure + // without the flags and let checkV8Flags warn. + delete process.env[RESTARTED_PID_ENV]; + console.warn( + `[CodSpeed] failed to restart node with the required V8 flags: ${String(error)}`, + ); + } +}; + export const checkV8Flags = () => { const requiredFlags = getV8Flags(); const actualFlags = process.execArgv; diff --git a/packages/core/src/native_core/index.ts b/packages/core/src/native_core/index.ts index 4e64f654..8c5e5c3b 100644 --- a/packages/core/src/native_core/index.ts +++ b/packages/core/src/native_core/index.ts @@ -5,6 +5,7 @@ import { LinuxPerf } from "./linux_perf/linux_perf"; interface NativeCore { InstrumentHooks: InstrumentHooks; LinuxPerf: typeof LinuxPerf; + restart(executable: string, args: string[]): never; } interface NativeCoreWithBindingStatus extends NativeCore { @@ -26,6 +27,11 @@ try { logDebug("Failed to bind native core, instruments will not work."); logDebug(e); native_core = { + restart: () => { + throw new Error( + `[CodSpeed] Cannot restart Node.js because the native core module is not bound: ${String(e)}`, + ); + }, LinuxPerf: class LinuxPerf { start() { return false; diff --git a/packages/core/src/native_core/native_core.cc b/packages/core/src/native_core/native_core.cc index aa1e5c71..69010178 100644 --- a/packages/core/src/native_core/native_core.cc +++ b/packages/core/src/native_core/native_core.cc @@ -2,11 +2,143 @@ #include "instruments/hooks_wrapper.h" #include +#include +#include +#include +#include + +#if !defined(_WIN32) +#include +#include +#endif + namespace codspeed_native { +namespace { + +bool ContainsNulByte(const std::string &value) { + return value.find('\0') != std::string::npos; +} + +#if !defined(_WIN32) +// `execv` closes descriptors marked close-on-exec, and libuv marks the stdio +// descriptors that way. Clear the flag like Node's own `process.execve` does; +// otherwise the restarted process writes to /dev/null. +int PersistStandardStream(int fd) { + const int flags = fcntl(fd, F_GETFD, 0); + if (flags < 0) { + return flags; + } + return fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC); +} +#endif + +// Replaces the process image via `execv`, keeping the PID, environment and open +// descriptors. Throws on failure; the current image stays usable. +Napi::Value Restart(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + + if (info.Length() != 2) { + Napi::TypeError::New(env, "Expected 2 arguments: executable and args") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + if (!info[0].IsString()) { + Napi::TypeError::New(env, "Expected string (executable)") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + if (!info[1].IsArray()) { + Napi::TypeError::New(env, "Expected array of strings (args)") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + const std::string executable = info[0].As().Utf8Value(); + if (executable.empty()) { + Napi::TypeError::New(env, "Expected non-empty string (executable)") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + if (ContainsNulByte(executable)) { + Napi::TypeError::New(env, "executable must not contain NUL bytes") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + const Napi::Array args = info[1].As(); + const uint32_t argc = args.Length(); + if (argc == 0) { + Napi::TypeError::New(env, "Expected at least argv[0] in args: pass the " + "complete argv, including the program name") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + // Fill the strings before taking any pointer out of them: `execv` needs + // stable storage, so the vector must not reallocate afterwards. + std::vector arg_strings; + arg_strings.reserve(argc); + for (uint32_t i = 0; i < argc; i++) { + const Napi::Value arg = args.Get(i); + if (!arg.IsString()) { + Napi::TypeError::New(env, "Expected string (args[" + std::to_string(i) + + "])") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + arg_strings.push_back(arg.As().Utf8Value()); + if (ContainsNulByte(arg_strings.back())) { + Napi::TypeError::New(env, "args[" + std::to_string(i) + + "] must not contain NUL bytes") + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + } + +#if defined(_WIN32) + Napi::Error::New(env, "restart() is not supported on Windows: pass the " + "required Node.js flags from the launcher") + .ThrowAsJavaScriptException(); + return env.Undefined(); +#else + std::vector argv; + argv.reserve(arg_strings.size() + 1); + for (std::string &arg : arg_strings) { + argv.push_back(const_cast(arg.c_str())); + } + argv.push_back(nullptr); + + if (PersistStandardStream(0) < 0 || PersistStandardStream(1) < 0 || + PersistStandardStream(2) < 0) { + const int fcntl_errno = errno; + Napi::Error::New(env, "Failed to keep stdio open across restart: " + + std::string(std::strerror(fcntl_errno))) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + execv(executable.c_str(), argv.data()); + + // Reaching this point means the exec failed and the current process is still + // running. Capture errno before any further N-API call can clobber it. + const int exec_errno = errno; + Napi::Error::New(env, "Failed to restart with execv(\"" + executable + + "\"): " + std::strerror(exec_errno)) + .ThrowAsJavaScriptException(); + return env.Undefined(); +#endif +} + +} // namespace + Napi::Object Initialize(Napi::Env env, Napi::Object exports) { codspeed_native::LinuxPerf::Initialize(env, exports); codspeed_native::instruments::hooks_wrapper::Initialize(env, exports); + exports.Set(Napi::String::New(env, "restart"), + Napi::Function::New(env, Restart)); return exports; } diff --git a/packages/core/tests/ensureV8Flags.integ.test.ts b/packages/core/tests/ensureV8Flags.integ.test.ts new file mode 100644 index 00000000..b76dc97b --- /dev/null +++ b/packages/core/tests/ensureV8Flags.integ.test.ts @@ -0,0 +1,109 @@ +/* eslint-disable @typescript-eslint/no-require-imports */ +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +export {}; // Make this a module + +interface BenchmarkProcess { + spawnedPid: number | undefined; + pid: number; + execArgv: string[]; + stderr: string; +} + +const runBenchmarkProcess = ( + env: NodeJS.ProcessEnv, + nodeArgs: string[] = [], + prelude = "", +): BenchmarkProcess => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "codspeed-ensure-v8-")); + const script = path.join(dir, "bench.js"); + fs.writeFileSync( + script, + `${prelude} +const core = require(${JSON.stringify(require.resolve(".."))}); +core.ensureV8Flags(); +process.stdout.write( + JSON.stringify({ pid: process.pid, execArgv: process.execArgv }), +); +`, + ); + try { + const baseEnv = { ...process.env }; + delete baseEnv.CODSPEED_ENV; + delete baseEnv.CODSPEED_RUNNER_MODE; + const result = spawnSync(process.execPath, [...nodeArgs, script], { + env: { ...baseEnv, ...env }, + encoding: "utf8", + }); + if (result.status !== 0) { + throw new Error(`child failed: ${result.stderr}`); + } + return { + spawnedPid: result.pid, + stderr: result.stderr, + ...(JSON.parse(result.stdout) as { pid: number; execArgv: string[] }), + }; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}; + +describe("ensureV8Flags", () => { + it("restarts the process with the required flags when they are missing", () => { + const { spawnedPid, pid, execArgv } = runBenchmarkProcess({ + CODSPEED_ENV: "runner", + CODSPEED_RUNNER_MODE: "instrumentation", + }); + + // Same pid: the process was replaced, not started as a child. + expect(pid).toBe(spawnedPid); + expect(execArgv.filter((flag) => flag === "--no-opt")).toHaveLength(1); + expect(execArgv).toContain("--predictable"); + }); + + it("adds the flags before the user's so user flags win", () => { + const { execArgv } = runBenchmarkProcess( + { CODSPEED_ENV: "runner", CODSPEED_RUNNER_MODE: "instrumentation" }, + ["--max-old-space-size=8192"], + ); + + expect(execArgv.indexOf("--max-old-space-size=4096")).toBeLessThan( + execArgv.indexOf("--max-old-space-size=8192"), + ); + }); + + it("warns and continues when the restart fails", () => { + // Unlinking the running executable makes /proc/self/exe point at a path + // that no longer exists, so execv fails with ENOENT. + const { execArgv, stderr } = runBenchmarkProcess( + { CODSPEED_ENV: "runner", CODSPEED_RUNNER_MODE: "instrumentation" }, + [], + `const fs = require("fs"), path = require("path"); +const copy = path.join(__dirname, "node-copy"); +if (!process.env.__NODE_COPY_RUNNING__) { + fs.copyFileSync(process.execPath, copy); + fs.chmodSync(copy, 0o755); + const r = require("child_process").spawnSync(copy, process.argv.slice(1), { + stdio: "inherit", + env: { ...process.env, __NODE_COPY_RUNNING__: "1" }, + }); + process.exit(r.status ?? 1); +} +fs.unlinkSync(copy);`, + ); + + expect(execArgv).not.toContain("--no-opt"); + expect(stderr).toContain("failed to restart node"); + }); + + it("keeps the process when CodSpeed is disabled", () => { + const { execArgv } = runBenchmarkProcess({ + CODSPEED_RUNNER_MODE: "instrumentation", + }); + + expect(execArgv).not.toContain("--no-opt"); + }); +}); diff --git a/packages/tinybench-plugin/src/index.ts b/packages/tinybench-plugin/src/index.ts index 298f97b9..7855a783 100644 --- a/packages/tinybench-plugin/src/index.ts +++ b/packages/tinybench-plugin/src/index.ts @@ -1,4 +1,5 @@ import { + ensureV8Flags, getCallingFile, getCodspeedRunnerMode, getInstrumentMode, @@ -16,6 +17,8 @@ import { getOrCreateUriMap } from "./uri"; import { setupCodspeedWalltimeBench } from "./walltime"; tryIntrospect(); +// Must run before any benchmark state exists: it may replace this process. +ensureV8Flags(); export function withCodSpeed(bench: Bench): Bench { const codspeedRunnerMode = getCodspeedRunnerMode(); From 3a489b4b0b8645ed6d08b6584afbbee74a777004 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 30 Oct 2026 18:40:26 +0100 Subject: [PATCH 3/5] ci: run the examples without the runner node wrapper in every mode The existing jobs reach node through PATH, so the runner wrapper always injects the V8 flags and the plugin's restart never runs. Launch node by absolute path, as Bazel's rules_js does, in simulation, walltime and memory mode, and fail when the plugin reports missing flags or a failed restart. Refs COD-4038 --- .github/workflows/ci.yml | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2bd82c02..681b9dd7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,3 +121,46 @@ jobs: env: CODSPEED_SKIP_UPLOAD: true CODSPEED_DEBUG: true + + # Bazel's rules_js runs node by absolute path, so the runner's `node` wrapper + # on PATH never injects the V8 flags and the plugin must restart itself. + restart-without-node-wrapper: + runs-on: "ubuntu-latest" + name: "restart without node wrapper (${{ matrix.mode }})" + strategy: + matrix: + mode: [simulation, walltime, memory] + fail-fast: false + steps: + - uses: "actions/checkout@v4" + with: + fetch-depth: 0 + submodules: true + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v6 + with: + cache: pnpm + node-version: 24 + - name: Restore turbo cache + uses: ./.github/actions/turbo-cache + with: + key-suffix: node24 + - run: pnpm install --frozen-lockfile --prefer-offline + - run: pnpm turbo run build + # Resolved before the action prefixes PATH with its wrapper. + - run: echo "REAL_NODE=$(command -v node)" >> "$GITHUB_ENV" + - uses: CodSpeedHQ/action@main + with: + mode: ${{ matrix.mode }} + working-directory: examples/with-javascript-esm + run: | + set -o pipefail + "$REAL_NODE" tinybench.js 2>&1 | tee tinybench.log + "$REAL_NODE" benchmark-js.js 2>&1 | tee benchmark-js.log + env: + CODSPEED_SKIP_UPLOAD: true + CODSPEED_DEBUG: true + - name: Check the benchmarks ran with the V8 flags + working-directory: examples/with-javascript-esm + run: | + ! grep -E "missing required flags|failed to restart node" tinybench.log benchmark-js.log From aa064a1ae80012a91f538051162534eb33204799 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 30 Oct 2026 18:46:10 +0100 Subject: [PATCH 4/5] ci: limit the GITHUB_TOKEN to read-only contents --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 681b9dd7..2021aba3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: pull_request: workflow_dispatch: +permissions: + contents: read + jobs: check: strategy: From 02e307470b7f00535f33bf58ea5af12a26decbc9 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 30 Oct 2026 18:55:26 +0100 Subject: [PATCH 5/5] fix(core): skip the V8 flag restart for stdin scripts A script piped into node (`node -`, or node without a script argument) is consumed before the plugin loads. The restarted process then read an empty stdin and exited successfully without running any benchmark. Keep the process and its missing-flags warning in that case. `-e`/`-p` code is part of execArgv and still restarts. --- packages/core/src/introspection.ts | 21 ++++++++++ .../core/tests/ensureV8Flags.integ.test.ts | 40 ++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/packages/core/src/introspection.ts b/packages/core/src/introspection.ts index 982ab2c8..979fd9af 100644 --- a/packages/core/src/introspection.ts +++ b/packages/core/src/introspection.ts @@ -94,6 +94,23 @@ export const tryIntrospect = () => { } }; +const EVAL_FLAGS = ["-e", "--eval", "-p", "--print"]; + +// `node -` or `node` without a script reads the script from stdin (or opens the +// REPL). The input is consumed by then, so the restarted process would get none. +const readsScriptFromStdin = () => { + const scriptPath = process.argv[1]; + if (scriptPath === "-") { + return true; + } + if (scriptPath !== undefined) { + return false; + } + return !process.execArgv.some((arg) => + EVAL_FLAGS.some((flag) => arg === flag || arg.startsWith(`${flag}=`)), + ); +}; + /** * Restarts the process with the V8 flags the runner mode requires when the * launcher did not pass them (e.g. Bazel executes node by absolute path, so the @@ -121,6 +138,10 @@ export const ensureV8Flags = () => { return; } + if (readsScriptFromStdin()) { + return; + } + // The restart did not help (its launcher stripped the flags again): do not // restart in a loop. if (process.env[RESTARTED_PID_ENV] === String(process.pid)) { diff --git a/packages/core/tests/ensureV8Flags.integ.test.ts b/packages/core/tests/ensureV8Flags.integ.test.ts index b76dc97b..208758f9 100644 --- a/packages/core/tests/ensureV8Flags.integ.test.ts +++ b/packages/core/tests/ensureV8Flags.integ.test.ts @@ -6,6 +6,12 @@ import path from "path"; export {}; // Make this a module +enum ScriptEntry { + File, + Stdin, + Eval, +} + interface BenchmarkProcess { spawnedPid: number | undefined; pid: number; @@ -17,6 +23,7 @@ const runBenchmarkProcess = ( env: NodeJS.ProcessEnv, nodeArgs: string[] = [], prelude = "", + entry = ScriptEntry.File, ): BenchmarkProcess => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "codspeed-ensure-v8-")); const script = path.join(dir, "bench.js"); @@ -34,9 +41,15 @@ process.stdout.write( const baseEnv = { ...process.env }; delete baseEnv.CODSPEED_ENV; delete baseEnv.CODSPEED_RUNNER_MODE; - const result = spawnSync(process.execPath, [...nodeArgs, script], { + const entryArgs = { + [ScriptEntry.File]: [script], + [ScriptEntry.Stdin]: ["-"], + [ScriptEntry.Eval]: ["-e", `require(${JSON.stringify(script)})`], + }[entry]; + const result = spawnSync(process.execPath, [...nodeArgs, ...entryArgs], { env: { ...baseEnv, ...env }, encoding: "utf8", + input: entry === ScriptEntry.Stdin ? fs.readFileSync(script) : undefined, }); if (result.status !== 0) { throw new Error(`child failed: ${result.stderr}`); @@ -64,6 +77,31 @@ describe("ensureV8Flags", () => { expect(execArgv).toContain("--predictable"); }); + it("restarts a process started with -e", () => { + const { spawnedPid, pid, execArgv } = runBenchmarkProcess( + { CODSPEED_ENV: "runner", CODSPEED_RUNNER_MODE: "instrumentation" }, + [], + "", + ScriptEntry.Eval, + ); + + expect(pid).toBe(spawnedPid); + expect(execArgv).toContain("--no-opt"); + }); + + it("keeps a process that read its script from stdin", () => { + // Parsing the output proves the script ran instead of an empty restart. + const { spawnedPid, pid, execArgv } = runBenchmarkProcess( + { CODSPEED_ENV: "runner", CODSPEED_RUNNER_MODE: "instrumentation" }, + [], + "", + ScriptEntry.Stdin, + ); + + expect(pid).toBe(spawnedPid); + expect(execArgv).not.toContain("--no-opt"); + }); + it("adds the flags before the user's so user flags win", () => { const { execArgv } = runBenchmarkProcess( { CODSPEED_ENV: "runner", CODSPEED_RUNNER_MODE: "instrumentation" },