fix: engine C-API's active-engine/callback globals were not thread-local, letting concurrent engines cross-contaminate

Follow-up to verifying an external Tengo-comparison report's concurrency
concern: engine.zig's g_active_engine/write_callback/read_callback and
host_abi.zig's native_host_call_fn/native_host_call_ctx were plain
process-global vars, unlike the 8 internal Runtime.activate()-pinned state
pointers (chunk/globals/heap/vm/tasks/fs/net/http), which are already
threadlocal.

pushEngineState/popEngineState overwrite these 5 fields for the duration
of every engine_run/engine_call. Two threads calling into two different
engines concurrently (an officially-supported pattern per
docs/embedding.md) could interleave those writes, so one engine's script
ran with the other engine's write/read callback, or the other engine's
host-call function paired with the other engine's ctx pointer -- a real
type-confusion hazard, since callbacks @ptrCast/@aligncast that ctx back
to their own expected struct type.

Confirmed with a real two-thread repro (added as a regression test) before
fixing: it reliably produced wrong results and even corrupted an unrelated
later test in the same process. Fixed by making all 5 fields threadlocal.

Verified under standard, -Dpreset=stress, and -Dgc_stress=true builds.

Author: ganehag

CHANGELOG.md

@@ -4,6 +4,49 @@ This changelog tracks notable language/runtime changes by implementation date.
 
 ## 2026-08-21
 
+### Fix — cross-thread engine C-API race: wrong callback (and ctx) could serve the wrong engine
+
+Follow-up to an independent LLM's Tengo-vs-Gengo comparison, which flagged
+that `src/engine.zig`'s C-API layer holds process-global state distinct
+from the (already thread-local) internal `Runtime` state. Verified this is
+real and exploitable, not just theoretical.
+
+`engine.zig`'s `g_active_engine`/`write_callback`/`read_callback` and
+`host_abi.zig`'s `native_host_call_fn`/`native_host_call_ctx` were plain
+(non-thread-local) `var`s, overwritten by `pushEngineState`/`popEngineState`
+for the duration of every `engine_run`/`engine_call`. Two threads calling
+into two *different* engines concurrently — an officially-supported usage
+pattern per `docs/embedding.md` ("one runtime per thread... may be used
+independently, including concurrently from different threads") — could
+interleave those writes, so one engine's script ran with the *other*
+engine's write/read callback, or (via `host_abi.setNativeHostCall`) the
+other engine's host-call function paired with the other engine's `ctx` — a
+real type-confusion hazard, since a host callback `@ptrCast`/`@alignCast`s
+that `ctx` back to its own expected struct type.
+
+Confirmed with a real two-thread repro before fixing: two engines with
+distinct host-call contexts, hammered concurrently via `engine_run`/
+`engine_call`, reliably produced wrong results (one engine's calls computed
+using the other engine's context) — and even corrupted an unrelated,
+later-running test in the same process, demonstrating how far the
+contamination reaches. All 8 of the *internal* `Runtime.activate()`-pinned
+state pointers (chunk/globals/heap/vm/tasks/fs/net/http) were already
+`threadlocal`, so this was scoped to exactly these 5 fields. Fixed by
+making all 5 `threadlocal`, matching the existing pattern; added a
+regression test (`engine.zig`, two real OS threads racing two engines'
+host-call callbacks) that fails reliably on the old code and passes
+deterministically on the fix. Verified under standard, `-Dpreset=stress`,
+and `-Dgc_stress=true` builds.
+
+Separately (not fixed, out of scope for this pass): `io.zig`'s own trace
+state (`g_trace_fn`/`g_trace_userdata`/`g_trace_handle`) is the same class
+of non-thread-local global, and `engine.zig`'s native test suite
+(`zig build engine-api-test`) already has one pre-existing, order-dependent
+flaky test (`engine_set_trace_fn fires per source line`) caused by it —
+confirmed present on a clean checkout, unrelated to this fix. That test
+suite is also not currently wired into `zig build test` or the pre-push
+hook, so it isn't exercised by CI at all.
+
 ### Audit — duplication/DRY sweep across native modules, VM arithmetic, and the compiler
 
 Four parallel audits (VM/opcode layer, native capability modules, compiler

src/engine.zig

@@ -42,12 +42,29 @@ const HostCallCallback = host_abi.NativeHostCallFn;
 // Active-engine state — set for the duration of engine_run / engine_call so that
 // write/read callbacks and capability handlers resolve to the calling engine's
 // per-instance configuration rather than a process-global slot.
-var g_active_engine: ?*Engine = null;
+//
+// threadlocal, not a plain process-global: the internal Runtime state these
+// wrap (chunk/globals/heap/vm/tasks/fs/net/http, all pinned by
+// Runtime.activate() below) is already threadlocal for exactly this reason.
+// Before this was threadlocal, two threads calling engine_run/engine_call on
+// two DIFFERENT engines concurrently could interleave pushEngineState's
+// writes, so one engine's script ran with the OTHER engine's write/read
+// callback (or, via host_abi.setNativeHostCall below, the other engine's
+// host-call function AND ctx) for the remainder of its execution — silent
+// misrouting at best (output/reads crossing engines), real type confusion
+// at worst (a host callback invoked with a foreign ctx it @ptrCast/@alignCast
+// s to its own expected type). Confirmed via a real two-thread repro (see
+// "two engines' host-call callbacks stay isolated..." below) before this
+// fix: it failed, and even corrupted an unrelated later test in the same
+// process. docs/embedding.md already promises "one runtime per thread...
+// active-runtime tracking is thread-local" — this makes that true for the
+// whole C-API surface, not just the internal Runtime layer.
+threadlocal var g_active_engine: ?*Engine = null;
 
 // Process-global active slots — overwritten by pushEngineState for each ABI
 // operation and restored by popEngineState on return.
-var write_callback: ?WriteCallback = null;
-var read_callback: ?ReadCallback = null;
+threadlocal var write_callback: ?WriteCallback = null;
+threadlocal var read_callback: ?ReadCallback = null;
 
 const ImportLoaderFn = *const fn (
     ctx: ?*anyopaque,
@@ -1816,6 +1833,88 @@ test "native host modules dispatch through the registered callback" {
     try std.testing.expectEqual(@as(usize, 2), context.module_calls);
 }
 
+const HostCallRaceContext = struct {
+    factor: i64,
+
+    fn callback(context: ?*anyopaque, id: u16, args: [*]const ValueWire, argc: u16, out: *ValueWire) callconv(.c) i32 {
+        const self = @as(*@This(), @ptrCast(@alignCast(context.?)));
+        switch (id) {
+            @intFromEnum(host_abi.HostCall.abi_version) => {
+                out.* = .{ .tag = @intFromEnum(WireTag.number), .flags = 0, .reserved = 0, .payload = @bitCast(@as(f64, @floatFromInt(host_abi.ABI_VERSION))), .len = 0, .reserved2 = 0 };
+                return @intFromEnum(host_abi.CallStatus.ok);
+            },
+            HostModuleCallIdBase => {
+                if (argc != 1 or args[0].tag != @intFromEnum(WireTag.number) or (args[0].flags & host_abi.FLAG_INTEGER) == 0) return @intFromEnum(host_abi.CallStatus.bad_args);
+                const value: i64 = @bitCast(args[0].payload);
+                out.* = .{ .tag = @intFromEnum(WireTag.number), .flags = host_abi.FLAG_INTEGER, .reserved = 0, .payload = @bitCast(value * self.factor), .len = 0, .reserved2 = 0 };
+                return @intFromEnum(host_abi.CallStatus.ok);
+            },
+            else => return @intFromEnum(host_abi.CallStatus.unsupported),
+        }
+    }
+};
+
+// Worker for the host-call cross-thread race test below: engine.zig's
+// g_active_engine/write_callback/read_callback (this file) and
+// host_abi.zig's native_host_call_fn/native_host_call_ctx are plain (not
+// threadlocal) globals, overwritten by pushEngineState/popEngineState for
+// the duration of every engine_run/engine_call. Two threads calling those
+// entry points on two DIFFERENT engines concurrently can interleave those
+// writes, so one engine's host-call callback runs with the OTHER engine's
+// ctx pointer — every call below multiplies by this worker's own `factor`,
+// so a wrong (foreign) ctx surfaces as a wrong product, not a crash.
+fn hostCallRaceWorker(handle: i32, factor: i64, iterations: usize, failed: *std.atomic.Value(bool)) void {
+    var context: HostCallRaceContext = .{ .factor = factor };
+    if (engine_set_host_call_fn(handle, HostCallRaceContext.callback, &context) != 0) {
+        failed.store(true, .seq_cst);
+        return;
+    }
+    const funcs = [_]HostModuleFuncDef{.{ .name_ptr = @intFromPtr("mul".ptr), .name_len = 3, .arity = 1 }};
+    if (engine_register_module(handle, @intFromPtr("m".ptr), 1, @intFromPtr(&funcs), funcs.len) != 0) {
+        failed.store(true, .seq_cst);
+        return;
+    }
+    const source =
+        \\m := import("host:m")
+        \\pub func answer(x int) int { return m.mul(x) }
+    ;
+    if (engine_run(handle, @intFromPtr(source.ptr), source.len) != 0) {
+        failed.store(true, .seq_cst);
+        return;
+    }
+    var i: usize = 0;
+    while (i < iterations) : (i += 1) {
+        var args = [_]ValueWire{.{ .tag = @intFromEnum(WireTag.number), .flags = host_abi.FLAG_INTEGER, .reserved = 0, .payload = @bitCast(@as(i64, 7)), .len = 0, .reserved2 = 0 }};
+        var out: ValueWire = undefined;
+        if (engine_call(handle, @intFromPtr("answer".ptr), 6, @intFromPtr(&args), 1, @intFromPtr(&out)) != 0) {
+            failed.store(true, .seq_cst);
+            return;
+        }
+        const got: i64 = @bitCast(out.payload);
+        if (got != 7 * factor) {
+            failed.store(true, .seq_cst);
+            return;
+        }
+    }
+}
+
+test "two engines' host-call callbacks stay isolated when engine_run/engine_call race across threads" {
+    if (comptime is_wasm) return;
+    const handleA = engine_init();
+    try std.testing.expect(handleA > 0);
+    defer engine_destroy(handleA);
+    const handleB = engine_init();
+    try std.testing.expect(handleB > 0);
+    defer engine_destroy(handleB);
+
+    var failed = std.atomic.Value(bool).init(false);
+    const t1 = try std.Thread.spawn(.{}, hostCallRaceWorker, .{ handleA, @as(i64, 2), @as(usize, 2000), &failed });
+    const t2 = try std.Thread.spawn(.{}, hostCallRaceWorker, .{ handleB, @as(i64, 3), @as(usize, 2000), &failed });
+    t1.join();
+    t2.join();
+    try std.testing.expect(!failed.load(.seq_cst));
+}
+
 test "engine_call: recover() in defer intercepts panic" {
     // Regression test: core.recover() inside a deferred function
     // must intercept panics even when the function is called via engine_call

src/runtime/host_abi.zig

@@ -40,8 +40,16 @@ pub const NativeHostCallFn = *const fn (
     out: *ValueWire,
 ) callconv(.c) i32;
 
-var native_host_call_fn: ?NativeHostCallFn = null;
-var native_host_call_ctx: ?*anyopaque = null;
+// threadlocal: set together by engine.zig's pushEngineState/popEngineState
+// for the duration of one engine_run/engine_call. As plain (non-threadlocal)
+// vars these two writes could tear across two threads calling into
+// different engines concurrently — engine B's callback function paired with
+// engine A's ctx pointer, a real type-confusion hazard since callbacks
+// @ptrCast/@alignCast ctx back to their own expected struct type. See
+// engine.zig's g_active_engine doc comment for the full writeup and the
+// two-thread repro that caught it.
+threadlocal var native_host_call_fn: ?NativeHostCallFn = null;
+threadlocal var native_host_call_ctx: ?*anyopaque = null;
 
 pub fn setNativeHostCall(fn_ptr: ?NativeHostCallFn, ctx: ?*anyopaque) void {
     native_host_call_fn = fn_ptr;