From e99eb60fd190772d258d49ff5db1efbbf70c20a3 Mon Sep 17 00:00:00 2001
From: Gabriel Cruz <8129788+gmelodie@users.noreply.github.com>
Date: Sat, 9 May 2026 10:47:37 -0300
Subject: [PATCH] chore: run tests with refc (#20)

* chore: run tests with refc

* chore: split ci jobs

* chore: fix tests
---
 .github/workflows/ci.yml   |  19 +++--
 ffi.nimble                 |  24 ++++--
 ffi/ffi_thread_request.nim |   2 +-
 tests/test_gc_compat.nim   | 165 +++++++++++++++++++++++++++++++++++++
 4 files changed, 195 insertions(+), 15 deletions(-)
 create mode 100644 tests/test_gc_compat.nim

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9ac0eb5..2874557 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -17,9 +17,10 @@ jobs:
       matrix:
         os: [ubuntu-22.04, macos-15, windows-latest]
         nim-version: ['2.2.4', 'stable']
+        mm: [orc, refc]
 
     runs-on: ${{ matrix.os }}
-    name: ${{ matrix.os }} / Nim ${{ matrix.nim-version }}
+    name: ${{ matrix.os }} / Nim ${{ matrix.nim-version }} / ${{ matrix.mm }}
 
     steps:
       - uses: actions/checkout@v4
@@ -68,18 +69,26 @@ jobs:
           fi
           nimble buildffi -y
 
-      - name: Run allocation tests
+      - name: Run allocation tests (${{ matrix.mm }})
         shell: bash
         run: |
           if [ "$RUNNER_OS" == "Windows" ]; then
             export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$HOME/.nimble/bin:$PATH"
           fi
-          nimble test_alloc -y
+          nim c -r --mm:${{ matrix.mm }} -d:chronicles_log_level=WARN tests/test_alloc.nim
 
-      - name: Run FFI context tests
+      - name: Run FFI context tests (${{ matrix.mm }})
         shell: bash
         run: |
           if [ "$RUNNER_OS" == "Windows" ]; then
             export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$HOME/.nimble/bin:$PATH"
           fi
-          nimble test_ffi -y
+          nim c -r --mm:${{ matrix.mm }} -d:chronicles_log_level=WARN tests/test_ffi_context.nim
+
+      - name: Run GC compatibility tests (${{ matrix.mm }})
+        shell: bash
+        run: |
+          if [ "$RUNNER_OS" == "Windows" ]; then
+            export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$HOME/.nimble/bin:$PATH"
+          fi
+          nim c -r --mm:${{ matrix.mm }} -d:chronicles_log_level=WARN tests/test_gc_compat.nim
diff --git a/ffi.nimble b/ffi.nimble
index 3086130..957d048 100644
--- a/ffi.nimble
+++ b/ffi.nimble
@@ -12,17 +12,23 @@ requires "chronos"
 requires "chronicles"
 requires "taskpools"
 
-const nimFlags = "--mm:orc -d:chronicles_log_level=WARN"
+const nimFlagsOrc = "--mm:orc -d:chronicles_log_level=WARN"
+const nimFlagsRefc = "--mm:refc -d:chronicles_log_level=WARN"
 
 task buildffi, "Compile the library":
-  exec "nim c " & nimFlags & " --app:lib --noMain ffi.nim"
+  exec "nim c " & nimFlagsOrc & " --app:lib --noMain ffi.nim"
 
-task test, "Run all tests":
-  exec "nim c -r " & nimFlags & " tests/test_alloc.nim"
-  exec "nim c -r " & nimFlags & " tests/test_ffi_context.nim"
+task test, "Run all tests under --mm:orc and --mm:refc":
+  for flags in [nimFlagsOrc, nimFlagsRefc]:
+    exec "nim c -r " & flags & " tests/test_alloc.nim"
+    exec "nim c -r " & flags & " tests/test_ffi_context.nim"
+    exec "nim c -r " & flags & " tests/test_gc_compat.nim"
 
-task test_alloc, "Run alloc unit tests":
-  exec "nim c -r " & nimFlags & " tests/test_alloc.nim"
 
-task test_ffi, "Run FFI context integration tests":
-  exec "nim c -r " & nimFlags & " tests/test_ffi_context.nim"
+task test_alloc, "Run alloc unit tests under --mm:orc and --mm:refc":
+  exec "nim c -r " & nimFlagsOrc & " tests/test_alloc.nim"
+  exec "nim c -r " & nimFlagsRefc & " tests/test_alloc.nim"
+
+task test_ffi, "Run FFI context integration tests under --mm:orc and --mm:refc":
+  exec "nim c -r " & nimFlagsOrc & " tests/test_ffi_context.nim"
+  exec "nim c -r " & nimFlagsRefc & " tests/test_ffi_context.nim"
diff --git a/ffi/ffi_thread_request.nim b/ffi/ffi_thread_request.nim
index 7cdda61..e1782e4 100644
--- a/ffi/ffi_thread_request.nim
+++ b/ffi/ffi_thread_request.nim
@@ -48,7 +48,7 @@ proc handleRes*[T: string | void](
 
   if res.isErr():
     foreignThreadGc:
-      let msg = "ffi error: handleRes fireSyncRes error: " & $res.error
+      let msg = res.error
       request[].callback(
         RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), request[].userData
       )
diff --git a/tests/test_gc_compat.nim b/tests/test_gc_compat.nim
new file mode 100644
index 0000000..f98ef61
--- /dev/null
+++ b/tests/test_gc_compat.nim
@@ -0,0 +1,165 @@
+## Verifies correct behaviour under both --mm:orc and --mm:refc.
+##
+## The foreignThreadGc template must set up / tear down the GC for foreign
+## threads under refc and be a no-op under orc/arc.  The handleRes proc
+## holds a string local long enough for the callback to read its cstring
+## -- these tests catch regressions in that lifetime guarantee.
+
+import std/locks
+import unittest2
+import results
+import ../ffi
+
+type GcTestLib = object
+
+type CallbackData = object
+  lock: Lock
+  cond: Cond
+  called: bool
+  retCode: cint
+  msg: array[1024, char]
+  msgLen: int
+
+proc initCallbackData(d: var CallbackData) =
+  d.lock.initLock()
+  d.cond.initCond()
+
+proc deinitCallbackData(d: var CallbackData) =
+  d.cond.deinitCond()
+  d.lock.deinitLock()
+
+proc testCallback(
+    retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer
+) {.cdecl, gcsafe, raises: [].} =
+  let d = cast[ptr CallbackData](userData)
+  acquire(d[].lock)
+  d[].retCode = retCode
+  let n = min(int(len), d[].msg.high)
+  if n > 0 and not msg.isNil:
+    copyMem(addr d[].msg[0], msg, n)
+  d[].msg[n] = '\0'
+  d[].msgLen = n
+  d[].called = true
+  signal(d[].cond)
+  release(d[].lock)
+
+proc waitCallback(d: var CallbackData) =
+  acquire(d.lock)
+  while not d.called:
+    wait(d.cond, d.lock)
+  release(d.lock)
+
+proc callbackMsg(d: var CallbackData): string =
+  result = newString(d.msgLen)
+  if d.msgLen > 0:
+    copyMem(addr result[0], addr d.msg[0], d.msgLen)
+
+# Concatenates GC-allocated strings so the result is not a string literal;
+# exercises the resStr lifetime binding inside handleRes.
+registerReqFFI(StringLifetimeRequest, lib: ptr GcTestLib):
+  proc(input: cstring): Future[Result[string, string]] {.async.} =
+    let prefix = "lifetime:"
+    let suffix = $input
+    return ok(prefix & suffix)
+
+# Returns 512 bytes of repeating a-z to stress GC with a moderately large
+# allocation that must survive the cross-thread callback.
+registerReqFFI(LargeStringRequest, lib: ptr GcTestLib):
+  proc(): Future[Result[string, string]] {.async.} =
+    var s = newString(512)
+    for i in 0 ..< 512:
+      s[i] = char(ord('a') + (i mod 26))
+    return ok(s)
+
+# Error path: the error string must be alive when the callback fires.
+registerReqFFI(GcErrRequest, lib: ptr GcTestLib):
+  proc(input: cstring): Future[Result[string, string]] {.async.} =
+    return err("gc-err:" & $input)
+
+suite "foreignThreadGc template":
+  test "body executes under current --mm":
+    var executed = false
+    foreignThreadGc:
+      executed = true
+    check executed
+
+  test "body executes exactly once":
+    var count = 0
+    foreignThreadGc:
+      inc count
+    check count == 1
+
+suite "GC safety - string lifetime across thread boundary":
+  test "ok string result remains valid when callback fires":
+    var d: CallbackData
+    initCallbackData(d)
+    defer: deinitCallbackData(d)
+
+    let ctx = createFFIContext[GcTestLib]().valueOr:
+      checkpoint "createFFIContext failed: " & $error
+      check false
+      return
+    defer: discard destroyFFIContext(ctx)
+
+    check sendRequestToFFIThread(
+      ctx, StringLifetimeRequest.ffiNewReq(testCallback, addr d, "hello".cstring)
+    ).isOk()
+    waitCallback(d)
+    check d.retCode == RET_OK
+    check callbackMsg(d) == "lifetime:hello"
+
+  test "error string lifetime across thread boundary":
+    var d: CallbackData
+    initCallbackData(d)
+    defer: deinitCallbackData(d)
+
+    let ctx = createFFIContext[GcTestLib]().valueOr:
+      check false
+      return
+    defer: discard destroyFFIContext(ctx)
+
+    check sendRequestToFFIThread(
+      ctx, GcErrRequest.ffiNewReq(testCallback, addr d, "test".cstring)
+    ).isOk()
+    waitCallback(d)
+    check d.retCode == RET_ERR
+    check callbackMsg(d) == "gc-err:test"
+
+  test "large string result is delivered without corruption":
+    var d: CallbackData
+    initCallbackData(d)
+    defer: deinitCallbackData(d)
+
+    let ctx = createFFIContext[GcTestLib]().valueOr:
+      check false
+      return
+    defer: discard destroyFFIContext(ctx)
+
+    check sendRequestToFFIThread(
+      ctx, LargeStringRequest.ffiNewReq(testCallback, addr d)
+    ).isOk()
+    waitCallback(d)
+    check d.retCode == RET_OK
+    check d.msgLen == 512
+    check d.msg[0] == 'a'
+    check d.msg[25] == 'z'
+    check d.msg[26] == 'a'
+
+suite "GC stability - repeated requests":
+  test "20 sequential requests without GC corruption":
+    let ctx = createFFIContext[GcTestLib]().valueOr:
+      check false
+      return
+    defer: discard destroyFFIContext(ctx)
+
+    for i in 1 .. 20:
+      var d: CallbackData
+      initCallbackData(d)
+      let input = "iter" & $i
+      check sendRequestToFFIThread(
+        ctx, StringLifetimeRequest.ffiNewReq(testCallback, addr d, input.cstring)
+      ).isOk()
+      waitCallback(d)
+      deinitCallbackData(d)
+      check d.retCode == RET_OK
+      check callbackMsg(d) == "lifetime:" & input