diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9b11a..8906848 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,19 +46,19 @@ All notable changes to this project are documented in this file. where `-install_name` requires `-dynamiclib`. ### Added -- **`{.ffiStatic.}`**: exports a context-independent proc. It takes no library - param and its wrapper takes no `ctx`, so a host can call a stateless utility - (key generation, parsing, a version string) without constructing the library +- **`{.ffiStatic.}`**: exports a context-independent proc — no library param, and + no `ctx` in its wrapper, so a host can call a stateless utility (key generation, + parsing, a version string) without constructing the library ([#134](https://github.com/logos-messaging/nim-ffi/issues/134)). Wired for both the `cbor` and `c` ABIs across all four backends: the C header emits - `_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 — - `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. + `_static_(...)`, C++ and Rust 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 held for the rest of the + process, so that call starts a thread pair nothing tears down — + `destroyFFIContext` refuses it; `destroyStaticFFIContext` is the Nim-side + teardown for process shutdown and tests, with no foreign equivalent. 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/README.md b/README.md index 5e845bb..89ccd85 100644 --- a/README.md +++ b/README.md @@ -200,8 +200,13 @@ exports. In C++ and Rust a static is an associated function on the ctx type The handler still needs an FFI thread, so it runs on the library's **static context**: created on the first `{.ffiStatic.}` call, then alive for the rest of -the process — no ctx owns it, so nothing tears its thread pair down. It has no -`myLib`, which is why a static proc cannot take the library value. +the process — no ctx owns it, so nothing tears its thread pair down, and it holds +one of the pool's slots. It has no `myLib`, which is why a static proc cannot +take the library value. + +There is no foreign teardown for it. From Nim, `destroyStaticFFIContext(pool)` +stops the thread pair and frees the slot; it is only sound once nothing will call +a `{.ffiStatic.}` proc again, so it is meant for process shutdown and tests. The macro rejects an `{.ffiHandle.}` parameter or return: a handle is registered in the context that created it, which a static proc cannot reach. Under diff --git a/examples/echo/echo.nim b/examples/echo/echo.nim index b5746d8..e8f55f4 100644 --- a/examples/echo/echo.nim +++ b/examples/echo/echo.nim @@ -44,7 +44,7 @@ proc echoVersion*(e: Echo): Future[Result[string, string]] {.ffi.} = ## Returns the library's version string. return ok("nim-echo v0.1.0") -# The two below need no `Echo`, so their wrappers take no ctx. +# No `Echo` param, so the wrappers take no ctx. proc echoLibVersion*(): Future[Result[string, string]] {.ffiStatic.} = return ok("nim-echo v0.1.0") diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index fd26a92..4568594 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -683,8 +683,8 @@ proc emitProcWrapper( ctxType, libType, libName: string, m: FFIProcMeta, ) = - ## Reply trampoline + CBOR-encoding wrapper: `_ctx_` for a method, - ## `_static_` for a static. `_` is the raw dylib symbol. + ## Reply trampoline + wrapper: `_ctx_`, or `_static_` for a + ## static; `_` itself is the raw symbol the dylib exports. let isStatic = m.isStatic() let stripped = stripLibPrefix(m.procName, libName) let reqName = reqStructName(m) @@ -809,8 +809,8 @@ proc monomorphiseAll( discard ensureCType(reg, n) reqTypes.add(n) var respTypes: seq[string] = @[] - for m in replyProcs: - respTypes.add(cReturnType(reg, m)) + for p in replyProcs: + respTypes.add(cReturnType(reg, p)) for ev in events: discard ensureCType(reg, ev.payloadTypeName) return (reqTypes, respTypes) @@ -1329,7 +1329,6 @@ proc emitAbiProcWrapper( ctxType, libName, libType: string, m: FFIProcMeta, ) = - ## See `emitProcWrapper` for why a static's wrapper is `_static_`. let isStatic = m.isStatic() let stripped = stripLibPrefix(m.procName, m.libName) let reqStruct = reqStructName(m) diff --git a/ffi/codegen/c_cpp_common.nim b/ffi/codegen/c_cpp_common.nim index 6d734ba..3a26c8e 100644 --- a/ffi/codegen/c_cpp_common.nim +++ b/ffi/codegen/c_cpp_common.nim @@ -1,6 +1,6 @@ ## Helpers shared by the C/C++ binding generators (cpp.nim, c.nim). -import std/[strutils, options] +import std/strutils import ./meta, ./string_helpers proc stripLibPrefix*(procName, libName: string): string = diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index fd8806d..b806c68 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -506,8 +506,8 @@ proc generateCppHeader*( lines.add(" }") lines.add("") - # A static forwards its own `timeout`; a method captures `this` and calls - # `this->methodName(...)` so a same-named param can't shadow the call target. + # A method calls `this->methodName(...)` so a same-named param can't shadow + # the call target; a static has no `this` and forwards its own `timeout`. let staticArgs = if methParamNames.len > 0: methParamNamesStr & ", timeout" diff --git a/ffi/codegen/meta.nim b/ffi/codegen/meta.nim index b6af5da..a421547 100644 --- a/ffi/codegen/meta.nim +++ b/ffi/codegen/meta.nim @@ -148,7 +148,6 @@ proc isFFIEnumTypeName*(name: string): bool {.compileTime.} = name in ffiEnumTypeNames func isStatic*(p: FFIProcMeta): bool = - ## True for a `{.ffiStatic.}` proc: no library receiver, no ctx in its wrapper. p.kind == FFIKind.STATIC type ClassifiedProcs* = object @@ -175,7 +174,10 @@ func classifyProcs*(procs: seq[FFIProcMeta]): ClassifiedProcs = func dtorProcName*(c: ClassifiedProcs): string = ## The destructor's proc name, or "" when the library has no destructor. - if c.dtor.isSome(): c.dtor.get().procName else: "" + if c.dtor.isSome(): + c.dtor.get().procName + else: + "" func replyProcs*(c: ClassifiedProcs): seq[FFIProcMeta] = ## Procs that reply with a decoded value: methods and statics. diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 8c7ee42..eba0d13 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -186,7 +186,6 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = lines.add(renderMemberDocComment(p.doc)) case p.kind of FFIKind.FFI, FFIKind.STATIC: - # Method-style: ctx first. A static is the same shape, minus the ctx. if not p.isStatic(): params.add("ctx: *mut c_void") params.add("callback: FFICallback") diff --git a/ffi/ffi_context_pool.nim b/ffi/ffi_context_pool.nim index 5f324c4..6c0f62d 100644 --- a/ffi/ffi_context_pool.nim +++ b/ffi/ffi_context_pool.nim @@ -9,12 +9,14 @@ type ## Lifecycle of the pool's `{.ffiStatic.}` context; see `staticFFIContext`. StaticCtxNone StaticCtxCreating + StaticCtxDestroying StaticCtxReady FFIContextPool*[T] = object - ## Fixed pool. Each live context holds 5 ThreadSignalPtrs — one fd each on - ## Linux, two (a socketpair) elsewhere. Under refc a destroyed context cannot - ## close them (see `deinitContextResources`), so churn leaks fds unbounded. + ## Fixed pool of FFI contexts, plus the one `{.ffiStatic.}` context. + # Each live context holds 5 ThreadSignalPtrs — one fd each on Linux, two (a + # socketpair) elsewhere. Under refc a destroyed context cannot close them + # (see `deinitContextResources`), so churn leaks fds unbounded. slots: array[MaxFFIContexts, FFIContext[T]] inUse: array[MaxFFIContexts, Atomic[bool]] staticCtx: Atomic[pointer] @@ -44,9 +46,10 @@ proc createFFIContext*[T]( 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) + ## True while `ctx` is the pool's static context, including mid-teardown. + # `staticCtx` is cleared only once the slot is released, so matching on the + # pointer covers `Destroying` too. + pool.staticCtx.load() == cast[pointer](ctx) proc destroyFFIContext*[T]( pool: var FFIContextPool[T], ctx: ptr FFIContext[T] @@ -67,17 +70,16 @@ proc destroyFFIContext*[T]( proc staticFFIContext*[T]( pool: var FFIContextPool[T] ): Result[ptr FFIContext[T], string] = - ## The pool's `{.ffiStatic.}` context: a static proc has no ctx of its own, but - ## its handler still needs an FFI thread. Created on first use and never - ## destroyed, so it holds a slot for good and `pool` must outlive its threads — - ## only ever call this on the global `declareLibrary` emits. `myLib` stays the - ## zero value; a static handler must not touch it. A failed create resets to - ## `StaticCtxNone` so the spinning losers retry instead of hanging. + ## The pool's `{.ffiStatic.}` context, created on first use: a static proc has + ## no ctx of its own, but its handler still needs an FFI thread. + # Holds its slot until `destroyStaticFFIContext`, so `pool` must outlive its + # threads: only call this on the global `declareLibrary` emits. `myLib` stays + # the zero value. A failed create resets to `StaticCtxNone` so waiters retry. while true: case pool.staticState.load() of StaticCtxReady: return ok(cast[ptr FFIContext[T]](pool.staticCtx.load())) - of StaticCtxCreating: + of StaticCtxCreating, StaticCtxDestroying: cpuRelax() of StaticCtxNone: var expected = StaticCtxNone @@ -92,14 +94,16 @@ proc staticFFIContext*[T]( 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: + ## threads and frees its slot. A no-op when there is no static context. + # Claiming `Ready -> Destroying` serialises concurrent teardowns; it does not + # make teardown safe against a static call already in flight. + var expected = StaticCtxReady + if not pool.staticState.compareExchange(expected, StaticCtxDestroying): return ok() let ctx = cast[ptr FFIContext[T]](pool.staticCtx.load()) ctx.stopAndJoinThreads().isOkOr: + # Threads are still live: leak the slot rather than free resources under them. + pool.staticState.store(StaticCtxReady) return err("destroyStaticFFIContext: " & $error) let deinitRes = ctx.deinitContextResources() pool.releaseSlot(ctx) @@ -107,7 +111,7 @@ proc destroyStaticFFIContext*[T](pool: var FFIContextPool[T]): Result[void, stri pool.staticState.store(StaticCtxNone) deinitRes.isOkOr: return err("destroyStaticFFIContext: " & $error) - return ok() + ok() proc isValidCtx*[T](pool: var FFIContextPool[T], ctx: pointer): bool = ## Rejects nil / dangling pointers at the API boundary. diff --git a/ffi/internal/c_macro_helpers.nim b/ffi/internal/c_macro_helpers.nim index 16fa5e1..a134f71 100644 --- a/ffi/internal/c_macro_helpers.nim +++ b/ffi/internal/c_macro_helpers.nim @@ -706,13 +706,48 @@ proc stringTrampBody(boxName: NimNode): NimNode = except CatchableError as e: box.fn(RET_ERR, "".cstring, e.msg.cstring, box.ud) -proc exportedMethodProc( - spec: CAbiSpec, boxName, envWire, trampName, poolIdent, cbType: NimNode +proc ctxBindingGuard( + poolIdent, emptyReply, ctxIdent: NimNode, isStatic: bool +): NimNode {.compileTime.} = + ## Prologue that binds `ctxIdent`: a method validates the ctx it was handed, a + ## static resolves the library's shared one. + if not isStatic: + return quote: + if onReply.isNil(): + return RET_MISSING_CALLBACK + if not `poolIdent`.isValidCtx(cast[pointer](`ctxIdent`)): + onReply( + RET_ERR, `emptyReply`, "ctx is not a valid FFI context".cstring, userData + ) + return RET_ERR + let guard = quote: + if onReply.isNil(): + return RET_MISSING_CALLBACK + let `ctxIdent` = `poolIdent`.staticFFIContext().valueOr: + let errStr = "ffiStatic: " & error + onReply(RET_ERR, `emptyReply`, errStr.cstring, userData) + return RET_ERR + # A static call may be the host's first entry into the library. Raw AST, not + # `quote`: `when declared` over an undeclared symbol inside `quote` ICEs. + guard.insert( + 0, + nnkWhenStmt.newTree( + nnkElifBranch.newTree( + newCall(ident("declared"), ident("initializeLibrary")), + newStmtList(newCall(ident("initializeLibrary"))), + ) + ), + ) + guard + +proc exportedProc( + spec: CAbiSpec, + boxName, envWire, trampName, poolIdent, cbType: NimNode, + isStatic: bool, ): NimNode = # No `foreignThreadGc`: `cwireUnpack`/`cwirePack` alloc on the calling thread (already GC-registered); wrapping would free its live ORC heap. let envName = spec.envelope - let libFFICtx = - nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), spec.libType)) + let ctxIdent = ident("ctx") # String reply: empty non-nil cstring on error; object reply: nil ptr gated by err_code. let emptyReply = if isStringType(spec.respType): @@ -720,11 +755,6 @@ proc exportedMethodProc( else: newNilLit() let body = quote: - if onReply.isNil(): - return RET_MISSING_CALLBACK - if not `poolIdent`.isValidCtx(cast[pointer](ctx)): - onReply(RET_ERR, `emptyReply`, "ctx is not a valid FFI context".cstring, userData) - return RET_ERR var ownedWire: `envWire` cwirePack(ownedWire, cwireUnpack(req[])) let ownedCopy = cwireOwnedCopy(ownedWire) @@ -742,7 +772,7 @@ proc exportedMethodProc( ) let sendRes = try: - ffi_context.sendRequestToFFIThread(ctx, reqPtr) + ffi_context.sendRequestToFFIThread(`ctxIdent`, reqPtr) except Exception as e: Result[void, string].err("sendRequestToFFIThread exception: " & e.msg) if sendRes.isErr(): @@ -754,86 +784,25 @@ proc exportedMethodProc( return RET_ERR return RET_OK - newProc( - name = ident($envName & "CAbiExport"), - params = @[ - ident("cint"), - newIdentDefs(ident("ctx"), libFFICtx), - newIdentDefs(ident("onReply"), cbType), - newIdentDefs(ident("userData"), ident("pointer")), - newIdentDefs(ident("req"), nnkPtrTy.newTree(envWire)), - ], - body = body, - pragmas = nnkPragma.newTree( - ident("dynlib"), - nnkExprColonExpr.newTree(ident("exportc"), newStrLitNode(spec.exportName)), - ident("cdecl"), - nnkExprColonExpr.newTree(ident("raises"), nnkBracket.newTree()), - ), - ) + let fullBody = ctxBindingGuard(poolIdent, emptyReply, ctxIdent, isStatic) + for stmt in body: + fullBody.add(stmt) + + var params = @[ + ident("cint"), + newIdentDefs(ident("onReply"), cbType), + newIdentDefs(ident("userData"), ident("pointer")), + newIdentDefs(ident("req"), nnkPtrTy.newTree(envWire)), + ] + if not isStatic: + let libFFICtx = + nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), spec.libType)) + params.insert(newIdentDefs(ctxIdent, libFFICtx), 1) -proc exportedStaticProc( - spec: CAbiSpec, boxName, envWire, trampName, poolIdent, cbType: NimNode -): NimNode = - ## Ctx-less twin of `exportedMethodProc`: binds the library's static context - ## instead of taking one. `initGuard` is raw AST because a `when declared` over - ## an undeclared symbol inside `quote` ICEs (see `exportedCtorProc`). - let envName = spec.envelope - let emptyReply = - if isStringType(spec.respType): - newDotExpr(newLit(""), ident("cstring")) - else: - newNilLit() - let initGuard = nnkWhenStmt.newTree( - nnkElifBranch.newTree( - newCall(ident("declared"), ident("initializeLibrary")), - newStmtList(newCall(ident("initializeLibrary"))), - ) - ) - let body = quote: - if onReply.isNil(): - return RET_MISSING_CALLBACK - let ctx = `poolIdent`.staticFFIContext().valueOr: - let errStr = "ffiStatic: " & error - onReply(RET_ERR, `emptyReply`, errStr.cstring, userData) - return RET_ERR - var ownedWire: `envWire` - cwirePack(ownedWire, cwireUnpack(req[])) - let ownedCopy = cwireOwnedCopy(ownedWire) - if ownedCopy.isNil(): - cwireFree(ownedWire) - onReply(RET_ERR, `emptyReply`, "out of memory".cstring, userData) - return RET_ERR - let reqBuf = cast[ptr UncheckedArray[byte]](ownedCopy) - let box = cast[ptr `boxName`](allocBox(sizeof(`boxName`))) - box.fn = onReply - box.ud = userData - let typeStr = $`envName` - let reqPtr = FFIThreadRequest.initFromOwnedShared( - `trampName`, box, typeStr.cstring, reqBuf, sizeof(`envWire`), rawReply = true - ) - let sendRes = - try: - ffi_context.sendRequestToFFIThread(ctx, reqPtr) - except Exception as e: - Result[void, string].err("sendRequestToFFIThread exception: " & e.msg) - if sendRes.isErr(): - # See exportedMethodProc: the rejected send freed the struct copy, not the - # field buffers `ownedWire` still aliases. - cwireFree(ownedWire) - onReply(RET_ERR, `emptyReply`, sendRes.error.cstring, userData) - return RET_ERR - return RET_OK - body.insert(0, initGuard) newProc( name = ident($envName & "CAbiExport"), - params = @[ - ident("cint"), - newIdentDefs(ident("onReply"), cbType), - newIdentDefs(ident("userData"), ident("pointer")), - newIdentDefs(ident("req"), nnkPtrTy.newTree(envWire)), - ], - body = body, + params = params, + body = fullBody, pragmas = nnkPragma.newTree( ident("dynlib"), nnkExprColonExpr.newTree(ident("exportc"), newStrLitNode(spec.exportName)), @@ -949,14 +918,15 @@ proc flushCAbiDispatch*(): NimNode {.compileTime.} = sink.add(replyTrampProc(trampName, stringTrampBody(boxName))) sink.add(exportedCtorProc(spec, boxName, envWire, trampName, poolIdent, cbType)) of cakMethod, cakStatic: - let emitExport = - if spec.kind == cakStatic: exportedStaticProc else: exportedMethodProc + let isStatic = spec.kind == cakStatic let rt = spec.respType if isStringType(rt): let cbType = cAbiCbType(ident("cstring")) sink.add(boxTypeDef(boxName, cbType)) sink.add(replyTrampProc(trampName, stringTrampBody(boxName))) - sink.add(emitExport(spec, boxName, envWire, trampName, poolIdent, cbType)) + sink.add( + exportedProc(spec, boxName, envWire, trampName, poolIdent, cbType, isStatic) + ) # `isKnownFFIType`, not just `nnkIdent`: a bare `int` is an ident too, and # would otherwise reach for a `int_CWire` companion that is never emitted. elif rt.kind == nnkIdent and isKnownFFIType($rt): @@ -964,7 +934,9 @@ proc flushCAbiDispatch*(): NimNode {.compileTime.} = let cbType = cAbiCbType(nnkPtrTy.newTree(respWire)) sink.add(boxTypeDef(boxName, cbType)) sink.add(replyTrampProc(trampName, objectTrampBody(boxName, respWire))) - sink.add(emitExport(spec, boxName, envWire, trampName, poolIdent, cbType)) + sink.add( + exportedProc(spec, boxName, envWire, trampName, poolIdent, cbType, isStatic) + ) else: error( "abi = c: unsupported response type for proc '" & spec.exportName & "': " & diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index 075408f..2915811 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -831,11 +831,13 @@ proc buildFFIProc( recvType = firstParam[1] firstIsHandle = isHandleType(recvType) if (firstIsHandle or isStatic) and currentLibType.len == 0: + let why = + if isStatic: " takes no library param" else: " has an {.ffiHandle.} receiver" error( - where & " proc " & $procName & " carries no library type but no library is " & - "declared; call declareLibrary(name, LibType) first" + where & " proc " & $procName & why & " but no library is declared; " & + "call declareLibrary(name, LibType) first" ) - # A static proc and a handle receiver both carry no library type, so fall back to the declared one. + # Neither carries a library type, so fall back to the declared one. let libTypeName = if firstIsHandle or isStatic: ident(currentLibType) @@ -960,9 +962,9 @@ proc buildFFIProc( return RET_ERR proc buildStaticCtxGuard(): NimNode = - ## Binds the library's static context: a static wrapper takes no `ctx`, and a - ## static call may be the host's first entry (hence `initializeLibrary`). - ## `ctxIdent` is substituted so the send below sees it (`quote` gensyms). + ## Binds the library's static context; a static call may be the host's first + ## entry, hence `initializeLibrary`. + # `ctxIdent` is substituted so the send below sees it (`quote` gensyms). let ctxIdent = ident("ctx") quote: initializeLibrary() @@ -1064,8 +1066,7 @@ proc buildFFIProc( let exportedParams = cExportedParams(ctxType, withCtx = not isStatic) let ffiBody = newStmtList() - # Flattened, not nested: the static guard's `let ctx` has to be a sibling of - # the send below for it to be in scope. + # Flattened: the guard's `let ctx` must be a sibling of the send to be in scope. let guard = if isStatic: buildStaticCtxGuard() @@ -1151,10 +1152,8 @@ macro ffi*(args: varargs[untyped]): untyped = return buildFFIProc(prc, abiFormat, isStatic = false) macro ffiStatic*(args: varargs[untyped]): untyped = - ## Context-independent twin of `{.ffi.}`: the proc takes no library receiver and - ## its C wrapper takes no `ctx`, so a host can call it without constructing the - ## library. Handlers still run on an FFI thread — the library's static context, - ## created on the first such call and alive for the rest of the process. + ## Context-independent `{.ffi.}`: no library receiver, and no `ctx` in the C + ## wrapper, so a host calls it without constructing the library. requireBeforeGenBindings("`.ffiStatic.`") requireLibraryDeclared("`.ffiStatic.`") let prc = args[^1] diff --git a/tests/e2e/c_abi/test_echo_c_abi.c b/tests/e2e/c_abi/test_echo_c_abi.c index 3747a8c..b9398a6 100644 --- a/tests/e2e/c_abi/test_echo_c_abi.c +++ b/tests/e2e/c_abi/test_echo_c_abi.c @@ -118,9 +118,8 @@ static void test_version(EchoCtx* ctx) { assert(strcmp(w.text_a, "nim-echo v0.1.0") == 0); } -/* {.ffiStatic.}: no EchoCtx, and the library's static context is created by - * this very call. Runs before make_ctx() so nothing else has initialised the - * Nim runtime first. */ +/* No EchoCtx: this call creates the library's static context. Runs before + * make_ctx() so nothing else has initialised the Nim runtime first. */ static void test_static_no_ctx(void) { ReplyWaiter v; memset(&v, 0, sizeof(v)); diff --git a/tests/e2e/cpp/test_timer_e2e.cpp b/tests/e2e/cpp/test_timer_e2e.cpp index 840afe3..27e8958 100644 --- a/tests/e2e/cpp/test_timer_e2e.cpp +++ b/tests/e2e/cpp/test_timer_e2e.cpp @@ -247,7 +247,7 @@ TEST(TimerE2E, CrossLibrary) { EXPECT_EQ(e.shouted, "X-ECHO: ASYNC-E"); } -// The whole point of {.ffiStatic.}: no EchoCtx is ever constructed here. +// No EchoCtx is constructed anywhere in this test. TEST(TimerE2E, StaticProcNeedsNoContext) { EXPECT_EQ(mustOk(EchoCtx::lib_version()), "nim-echo v0.1.0"); diff --git a/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim b/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim index 14c7dbb..9d9e3d2 100644 --- a/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim +++ b/tests/unit/fixtures/ffistatic_abi_c_scalar_fixture.nim @@ -1,6 +1,5 @@ -## Must fail: `{.ffiStatic.}` never rides the ctx-bound all-scalar fast path, so a -## scalar return has no `abi = c` reply shape. The error must say so, not die on an -## undeclared `int_CWire` (see tests/unit/test_ffistatic_reject.nim). +## Must fail: no `abi = c` reply shape for a static's scalar return, and the error +## must say so rather than die on an undeclared `int_CWire`. import ffi, chronos diff --git a/tests/unit/fixtures/ffistatic_ok_fixture.nim b/tests/unit/fixtures/ffistatic_ok_fixture.nim index 5146af4..4bccca5 100644 --- a/tests/unit/fixtures/ffistatic_ok_fixture.nim +++ b/tests/unit/fixtures/ffistatic_ok_fixture.nim @@ -1,4 +1,4 @@ -## Must compile: proves the rejections are about handles, not the static shape. +## Must compile: the same shapes, with no handle crossing a static's boundary. import ffi, chronos diff --git a/tests/unit/test_ffi_context.nim b/tests/unit/test_ffi_context.nim index 1d50069..41bdedf 100644 --- a/tests/unit/test_ffi_context.nim +++ b/tests/unit/test_ffi_context.nim @@ -109,15 +109,11 @@ registerReqFFI(HeavyRefAllocRequest, lib: ptr TestLib): await sleepAsync(10.milliseconds) return ok("heavy-done") -# Globals, as declareLibrary emits them: a static ctx is never destroyed, so its -# threads outlive any scope and the pool must outlive them. -# -# One pool, filled once, for every slot-accounting case below. Under refc -# `deinitContextResources` cannot close a context's five ThreadSignalPtrs (see -# there), so every context ever created leaks its fds — and macOS spends two per -# signal. A second 32-slot fill puts the suite over the 1024-fd limit. +# Global, as declareLibrary emits it: a static ctx's threads may outlive any scope. +# One pool for every slot-accounting case below — under refc a destroyed context +# can't close its five ThreadSignalPtrs, so a second 32-slot fill would put the +# suite over the 1024-fd limit. var staticPool: FFIContextPool[TestLib] -var filler: seq[ptr FFIContext[TestLib]] suite "FFIContextPool": test "create and destroy via pool succeeds": @@ -139,28 +135,39 @@ suite "FFIContextPool": check pool.destroyFFIContext(ctx2).isOk() check ctx1 == ctx2 + # Each static case tears its pool back down on every exit path: left running + # under refc the threads race later suites' allocation and GC (macOS SIGSEGV). test "staticFFIContext returns one shared context and refuses destruction": + defer: + check staticPool.destroyStaticFFIContext().isOk() let first = staticPool.staticFFIContext().valueOr: assert false, "staticFFIContext failed: " & $error return check staticPool.staticFFIContext().tryGet() == first - # Owns a real slot, not a context handed out on the side. + # Occupies a pool slot like any other context. check staticPool.isValidCtx(first) check staticPool.destroyFFIContext(first).isErr() - # Still live and still the same context, not a released slot. + # Still live, and still the same context. check staticPool.staticFFIContext().tryGet() == first test "pool exhaustion errors and leaves staticFFIContext retryable": + var filler: seq[ptr FFIContext[TestLib]] + defer: + check staticPool.destroyStaticFFIContext().isOk() + for c in filler: + check staticPool.destroyFFIContext(c).isOk() + + check staticPool.staticFFIContext().isOk() var c = staticPool.createFFIContext() while c.isOk(): filler.add(c.tryGet()) c = staticPool.createFFIContext() - # The static ctx holds a slot for good, so only MaxFFIContexts-1 were left. + # The static ctx holds a slot, so only MaxFFIContexts-1 were left. check filler.len == MaxFFIContexts - 1 check staticPool.createFFIContext().isErr() - # Drop the static ctx and hand its slot straight to a plain one, so the - # retry below has to fail on a genuinely full pool. + # Hand the static ctx's slot straight to a plain one, so the retry below has + # to fail on a genuinely full pool. check staticPool.destroyStaticFFIContext().isOk() let reclaimed = staticPool.createFFIContext().valueOr: assert false, "createFFIContext(pool) failed on the freed static slot: " & $error @@ -171,15 +178,6 @@ suite "FFIContextPool": check staticPool.destroyFFIContext(filler.pop()).isOk() check staticPool.staticFFIContext().isOk() - # 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 this pool, so stop its threads here. - test "static contexts tear down without outliving the suite": - check staticPool.destroyStaticFFIContext().isOk() - for c in filler: - check staticPool.destroyFFIContext(c).isOk() - filler.setLen(0) - test "requests are processed via pool context": var pool: FFIContextPool[TestLib] var d: CallbackData diff --git a/tests/unit/test_ffistatic_reject.nim b/tests/unit/test_ffistatic_reject.nim index c41c79d..fcf9b97 100644 --- a/tests/unit/test_ffistatic_reject.nim +++ b/tests/unit/test_ffistatic_reject.nim @@ -1,10 +1,6 @@ -## Asserts `{.ffiStatic.}` rejects what cannot cross a context-independent proc: -## an {.ffiHandle.} parameter or return (both are resolved against the context -## that owns them), and an `abi = c` scalar return (no reply shape without the -## ctx-bound fast path). -## -## Each fixture compiles in a child `nim check` so its expected failure is a test -## assertion, not this file's own compile error. +## Asserts `{.ffiStatic.}` rejects an {.ffiHandle.} param/return and an `abi = c` +## scalar return. Each fixture compiles in a child `nim check` so its expected +## failure is an assertion rather than this file's own compile error. import std/[os, osproc, strutils, compilesettings] import unittest2 diff --git a/tests/unit/zz_exit.nim b/tests/unit/zz_exit.nim deleted file mode 100644 index 59a476c..0000000 --- a/tests/unit/zz_exit.nim +++ /dev/null @@ -1,11 +0,0 @@ -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 deleted file mode 100644 index e9ffb93..0000000 --- a/tests/unit/zz_size.nim +++ /dev/null @@ -1,4 +0,0 @@ -import ffi -type TestLib = object -echo "sizeof(FFIContext[TestLib]) = ", sizeof(FFIContext[TestLib]) -echo "sizeof(FFIContextPool[TestLib]) = ", sizeof(FFIContextPool[TestLib])