From 1d13eddd0a48f2c763d234002309afa1bcfeb5ae Mon Sep 17 00:00:00 2001 From: Gabriel Cruz Date: Fri, 17 Jul 2026 09:22:34 -0300 Subject: [PATCH] chore: destroyFFIContext refuse static context --- CHANGELOG.md | 8 +++++--- ffi/ffi_context_pool.nim | 27 +++++++++++++++++++++++++++ tests/unit/test_ffi_context.nim | 19 +++++++++++++++++++ tests/unit/zz_exit.nim | 11 +++++++++++ tests/unit/zz_size.nim | 4 ++++ 5 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 tests/unit/zz_exit.nim create mode 100644 tests/unit/zz_size.nim diff --git a/CHANGELOG.md b/CHANGELOG.md index 927758a..ce9b11a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,9 +54,11 @@ All notable changes to this project are documented in this file. `_static_(...)`, while C++ and Rust emit an associated function on the ctx type taking the `timeout` a method reads from its ctx. Handlers run on the library's *static context*, created on the first such call and alive for the - rest of the process, so that call starts a thread pair that is never torn down. - An `{.ffiHandle.}` parameter or return is rejected at macro time: a handle - belongs to the context that created it, which a static proc cannot reach. + rest of the process, so that call starts a thread pair that is never torn down — + `destroyFFIContext` refuses it rather than releasing its slot; `destroyStaticFFIContext` + is the explicit teardown counterpart (stops the thread pair and frees the slot) for + process shutdown and tests. An `{.ffiHandle.}` parameter or return is rejected at macro + time: a handle belongs to the context that created it, which a static proc cannot reach. - `{.ffi.}` now accepts an `enum` type, emitting a native enum in every target (C `enum`, C++ `enum class`, Rust enum, CDDL string choice). Values cross the wire as the text `$value` yields — the associated string if declared, else the diff --git a/ffi/ffi_context_pool.nim b/ffi/ffi_context_pool.nim index 089aede..38fac57 100644 --- a/ffi/ffi_context_pool.nim +++ b/ffi/ffi_context_pool.nim @@ -41,10 +41,18 @@ proc createFFIContext*[T]( return err("createFFIContext: initContextResources failed: " & $error) ok(ctx) +proc isStaticCtx[T](pool: var FFIContextPool[T], ctx: ptr FFIContext[T]): bool = + ## `staticCtx` is published before the state flips, so `Ready` implies it is readable. + pool.staticState.load() == StaticCtxReady and + pool.staticCtx.load() == cast[pointer](ctx) + proc destroyFFIContext*[T]( pool: var FFIContextPool[T], ctx: ptr FFIContext[T] ): Result[void, string] = ## On thread-exit timeout the slot is leaked; closing live-thread resources is unsafe. + # Destroying it would release the slot while `staticState` still points at it. + if pool.isStaticCtx(ctx): + return err("destroyFFIContext(pool): the {.ffiStatic.} context outlives every ctx") ctx.stopAndJoinThreads().isOkOr: return err("destroyFFIContext(pool): " & $error) # Required: next acquisition would otherwise re-init a live lock (UB). @@ -80,6 +88,25 @@ proc staticFFIContext*[T]( pool.staticState.store(StaticCtxReady) return ok(ctx) +proc destroyStaticFFIContext*[T](pool: var FFIContextPool[T]): Result[void, string] = + ## Teardown counterpart to `staticFFIContext`: stops the static context's + ## threads and frees its slot. The static context is meant to live for the + ## whole process, so only call this once nothing will call `staticFFIContext` + ## again (e.g. test teardown) — a lingering static context otherwise keeps its + ## FFI/event threads running for the process lifetime. + if pool.staticState.load() != StaticCtxReady: + return ok() + let ctx = cast[ptr FFIContext[T]](pool.staticCtx.load()) + ctx.stopAndJoinThreads().isOkOr: + return err("destroyStaticFFIContext: " & $error) + let deinitRes = ctx.deinitContextResources() + pool.releaseSlot(ctx) + pool.staticCtx.store(nil) + pool.staticState.store(StaticCtxNone) + deinitRes.isOkOr: + return err("destroyStaticFFIContext: " & $error) + return ok() + proc isValidCtx*[T](pool: var FFIContextPool[T], ctx: pointer): bool = ## Rejects nil / dangling pointers at the API boundary. if ctx.isNil(): diff --git a/tests/unit/test_ffi_context.nim b/tests/unit/test_ffi_context.nim index 5c3e8b9..24919ae 100644 --- a/tests/unit/test_ffi_context.nim +++ b/tests/unit/test_ffi_context.nim @@ -163,6 +163,18 @@ suite "FFIContextPool": for c in ctxs: discard sharedPool.destroyFFIContext(c) + test "destroyFFIContext refuses the static context but not a plain one": + let staticCtx = sharedPool.staticFFIContext().valueOr: + assert false, "staticFFIContext failed: " & $error + return + check sharedPool.destroyFFIContext(staticCtx).isErr() + # Still live and still the same context, not a released slot. + check sharedPool.staticFFIContext().tryGet() == staticCtx + let plain = sharedPool.createFFIContext().valueOr: + assert false, "createFFIContext(pool) failed: " & $error + return + check sharedPool.destroyFFIContext(plain).isOk() + test "a failed create leaves staticFFIContext retryable": var ctxs: array[MaxFFIContexts, ptr FFIContext[TestLib]] for i in 0 ..< MaxFFIContexts: @@ -176,6 +188,13 @@ suite "FFIContextPool": for i in 1 ..< MaxFFIContexts: discard retryPool.destroyFFIContext(ctxs[i]) + # Static contexts hold FFI/event threads for the whole process. Left running + # under refc they race with later suites' allocation and GC (macOS SIGSEGV). + # No later case uses these pools, so stop their threads here. + test "static contexts tear down without outliving the suite": + check sharedPool.destroyStaticFFIContext().isOk() + check retryPool.destroyStaticFFIContext().isOk() + test "requests are processed via pool context": var pool: FFIContextPool[TestLib] var d: CallbackData diff --git a/tests/unit/zz_exit.nim b/tests/unit/zz_exit.nim new file mode 100644 index 0000000..59a476c --- /dev/null +++ b/tests/unit/zz_exit.nim @@ -0,0 +1,11 @@ +import results +import ffi +type TestLib = object +var sharedPool: FFIContextPool[TestLib] +when isMainModule: + let s = sharedPool.staticFFIContext().valueOr: + quit("static failed: " & error) + echo "static ctx: ", cast[uint](s) + # Simulate the rest of the suite doing light work, then exit with the + # static thread still running. + echo "exiting with static thread alive" diff --git a/tests/unit/zz_size.nim b/tests/unit/zz_size.nim new file mode 100644 index 0000000..e9ffb93 --- /dev/null +++ b/tests/unit/zz_size.nim @@ -0,0 +1,4 @@ +import ffi +type TestLib = object +echo "sizeof(FFIContext[TestLib]) = ", sizeof(FFIContext[TestLib]) +echo "sizeof(FFIContextPool[TestLib]) = ", sizeof(FFIContextPool[TestLib])