runtimeError leaks open upvalues — closures captured in aborted fibers read freed memory

[RealDeuce]

Summary

When a fiber aborts, runtimeError() in src/vm/wren_vm.c unwinds the caller chain without closing the fiber's open upvalues. closeUpvalues() runs on CODE_RETURN and CODE_CLOSE_UPVALUE, but not on the abort path. Closures created inside the aborted frame that survive (held by module-level statics, host callbacks, observer lists, etc.) keep upvalues whose value pointers still point INTO the dead fiber's stack. Once GC collects the dead fiber and its stack allocation is freed, every read through those upvalues returns whatever now lives at that address.

Reproducer (Wren 0.4.0 / wren-cli)

class H {
  static fns   { __fns      }
  static fns=(v) { __fns = v }
}
H.fns = []

var spawn = Fn.new {|tag|
  var captured = tag
  H.fns.add(Fn.new { captured })
  Fiber.abort("done with %(tag)")
}

var N = 200
for (i in 0...N) {
  Fiber.new { spawn.call(i) }.try()
  if (i % 5 == 4) System.gc()
}
System.gc()

var bad = 0
var first_bad = -1
var first_bad_val = null
for (i in 0...H.fns.count) {
  var got = H.fns[i].call()
  if (got != i) {
    bad = bad + 1
    if (first_bad == -1) {
      first_bad     = i
      first_bad_val = got
    }
  }
}
System.print("%(bad) of %(N) closures returned wrong value")
if (first_bad >= 0) {
  System.print("first mismatch: fn[%(first_bad)] returned %(first_bad_val)")
}

Expected

fn[i] returns i for all 200 closures. The captured tag should be hoisted into the upvalue's closed field before the dead
fiber's stack memory is reclaimed.

Actual (wren-cli 0.4.0, FreeBSD/amd64)

194 of 200 closures returned wrong value
first mismatch: fn[0] returned 195

The numbers shift between platforms / allocator behaviours, but on every run a large majority of closures return values that belonged to a later spawn invocation — clear evidence that fiber N's freed stack got recycled into fiber N+k's stack, and the leaked upvalue from N is now reading N+k's slot.

Severity

Memory-safety bug. The benign manifestation (above) is wrong return values. With richer captured types — objects, closures, lists — the dangling pointer eventually decodes as a tagged pointer to whatever now sits at that address, and the bytecode dispatches a method on it. We hit this in our embedder (SyncTERM, a Wren-scripted SSH terminal) as a SIGSEGV in runInterpreter():

* frame #0: runInterpreter at wren_vm.c:1038:18
  frame #1: wrenCall at wren_vm.c:1467:32
  frame #2: ... (host's hook dispatcher firing a leaked Hook.every)

args[0] was a NaN-tagged pointer to an OBJ_UPVALUE whose classObj was NULL (upvalues are initialised with initObj(..., NULL) because they're never first-class). The dispatch tried to read classObj->methods.count → SIGSEGV. The path: an event-loop fiber aborted via a defensive throw; a periodic timer hook had captured upvalues from its scope and stayed registered; each fire allocated a new dispatch fiber whose stack landed at the dead loop fiber's recycled address; the leaked upvalue's stale value pointer eventually decoded the new dispatch fiber's local-slot bytes as an upvalue-typed Value.

Root cause

src/vm/wren_vm.c:403:

static void runtimeError(WrenVM* vm)
{
  ASSERT(wrenHasError(vm->fiber), "Should only call this after an error.");
  ObjFiber* current = vm->fiber;
  Value error = current->error;

  while (current != NULL)
  {
    current->error = error;

    if (current->state == FIBER_TRY)
    {
      current->caller->stackTop[-1] = vm->fiber->error;
      vm->fiber = current->caller;
      return;
    }

    ObjFiber* caller = current->caller;
    current->caller = NULL;
    current = caller;
  }
  ...
}

Every other code path that ends a function's stack — CODE_RETURN (line 1215) and CODE_CLOSE_UPVALUE (line 1209) — calls closeUpvalues() before discarding stack frames. The abort path is the only path that doesn't.

Suggested fix

   while (current != NULL)
   {
     current->error = error;

+    // Close any open upvalues on the aborting fiber.  Without this,
+    // the fiber's stack is freed during a future GC pass while
+    // upvalues that captured slots from it still point INTO that
+    // freed memory; subsequent reads return garbage.
+    closeUpvalues(current, current->stack);
+
     if (current->state == FIBER_TRY)
     {
       current->caller->stackTop[-1] = vm->fiber->error;
       vm->fiber = current->caller;
       return;
     }

     ObjFiber* caller = current->caller;
     current->caller = NULL;
     current = caller;
   }

Passing current->stack (the lowest address in the fiber's stack array) closes every upvalue still open on the fiber
regardless of which frame it belongs to — correct for an abort, since every frame is being abandoned.

Verification

Applied the patch against deps/wren/src/vm/wren_vm.c in the wren-cli tree and rebuilt. The reproducer above now reports:

0 of 200 closures returned wrong value

Tested on FreeBSD 14.3 / amd64 with the wren-cli 0.4.0 source.