mirror of
https://github.com/logos-messaging/nim-ffi.git
synced 2026-08-05 14:33:13 +00:00
fix: reject {.ffi.} calls on an unconstructed ref library
When no {.ffiCtor.} has stored a library, the FFI thread points `myLib` at a
default-valued fallback so handlers always have something to bind. For an
`object` library that is a usable zero value and callers legitimately depend on
it (tests/unit/test_ffi_handle drives a context that never runs a ctor), but for
a `ref` library the default is `nil`: the user body received a nil ref and
faulted on its first field access. A failing ctor is the common way to get
there, since the C entry point hands back a live context before the ctor body
has run on the FFI thread.
Track whether a ctor actually stored a library and, for `ref` library types
only, reject such requests with a clear error instead of dispatching them. The
check sits in the generated handler rather than the C entry point, so it runs
behind any queued constructor — a host that fires a call without awaiting the
create callback still succeeds, as before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULz7Md52AF6PmqZeCmh8b7
This commit is contained in:
parent
11fd49c2dc
commit
8f3d5cf8c2
@ -39,6 +39,9 @@ type FFIContext*[T] = object
|
||||
recycleDoneSignal: ThreadSignalPtr
|
||||
# fired by the recycle handler once the lib is freed and the slot released;
|
||||
# the synchronous recycleFFIContext caller waits on it.
|
||||
libReady*: Atomic[bool]
|
||||
# False until a {.ffiCtor.} stores the library; until then `myLib` is the
|
||||
# FFI thread's default-valued fallback, which for a `ref` type is nil.
|
||||
ffiThread: Thread[(ptr FFIContext[T])]
|
||||
eventThread: Thread[(ptr FFIContext[T])]
|
||||
reqQueueBank: RequestQueueBank
|
||||
@ -131,6 +134,7 @@ proc initContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] =
|
||||
initHandleRegistry(ctx[].handles)
|
||||
initEventQueue(ctx[].eventQueue)
|
||||
ctx.ffiHeartbeat.store(0)
|
||||
ctx.libReady.store(false)
|
||||
ctx.eventQueueStuck.store(false)
|
||||
ctx.ffiThreadExited.store(false)
|
||||
ctx.staleWarnInterval = StaleWarnInterval
|
||||
|
||||
@ -93,6 +93,8 @@ const RecycleTimeout = 1500.milliseconds
|
||||
proc freeLib[T](ctx: ptr FFIContext[T]) {.gcsafe.} =
|
||||
## Releases the library object the ctor stored in ctx.myLib. Only owned libs
|
||||
## (createShared'd by a ctor) are freed; the worker's stack fallback is not.
|
||||
# A reused slot skips initContextResources, so the recycle path clears this.
|
||||
ctx.libReady.store(false)
|
||||
if not ctx.myLibOwned or ctx.myLib.isNil():
|
||||
ctx.myLib = nil
|
||||
return
|
||||
|
||||
@ -1044,27 +1044,16 @@ proc buildFFIProc(
|
||||
lambdaParams.add(newIdentDefs(ident(extraParamNames[i]), extraParamTypes[i]))
|
||||
|
||||
let helperCall = newTree(nnkCall, userProcName)
|
||||
if not firstIsHandle and not isStatic:
|
||||
let bindsLib = not firstIsHandle and not isStatic
|
||||
if bindsLib:
|
||||
let ctxMyLib = newDotExpr(newTree(nnkDerefExpr, ctxHandlerName), ident("myLib"))
|
||||
helperCall.add(newTree(nnkDerefExpr, ctxMyLib))
|
||||
for name in extraParamNames:
|
||||
helperCall.add(ident(name))
|
||||
|
||||
let lambdaBody = newStmtList()
|
||||
if not firstIsHandle:
|
||||
# Reject a request that reached the FFI thread without a `{.ffiCtor.}`
|
||||
# having stored a library — `myLibOwned` is what distinguishes that from
|
||||
# the worker's default-valued stack fallback. Only for `ref` library
|
||||
# types: an `object` fallback is a usable zero value callers may rely on,
|
||||
# but a `ref` one is `nil`, so the user body would deref nil on its first
|
||||
# field access. Emitted in the handler, i.e. on the FFI thread behind any
|
||||
# queued ctor, so calling without awaiting the create callback still works.
|
||||
lambdaBody.add quote do:
|
||||
when `libTypeName` is ref:
|
||||
if not `ctxHandlerName`[].myLibOwned:
|
||||
return err(
|
||||
"library is not initialized: the constructor failed or has not run yet"
|
||||
)
|
||||
if bindsLib:
|
||||
lambdaBody.add(buildLibReadyGuard(ctxHandlerName, libTypeName))
|
||||
let retValIdent = ident("retVal")
|
||||
lambdaBody.add quote do:
|
||||
let `retValIdent` = (await `helperCall`).valueOr:
|
||||
@ -1330,6 +1319,7 @@ proc buildCtorProcessFFIRequestProc(
|
||||
let myLibIdent = newDotExpr(newTree(nnkDerefExpr, ctxIdent), ident("myLib"))
|
||||
let myLibOwnedIdent = newDotExpr(newTree(nnkDerefExpr, ctxIdent), ident("myLibOwned"))
|
||||
let myLibRefdIdent = newDotExpr(newTree(nnkDerefExpr, ctxIdent), ident("myLibRefd"))
|
||||
let libReadyIdent = newDotExpr(newTree(nnkDerefExpr, ctxIdent), ident("libReady"))
|
||||
newBody.add quote do:
|
||||
`myLibIdent` = createShared(`libTypeName`)
|
||||
`myLibIdent`[] = `libValIdent`
|
||||
@ -1340,6 +1330,8 @@ proc buildCtorProcessFFIRequestProc(
|
||||
when `libTypeName` is ref:
|
||||
GC_ref(`myLibIdent`[])
|
||||
`myLibRefdIdent` = true
|
||||
# After the store, so an observer never sees the fallback.
|
||||
`libReadyIdent`.store(true)
|
||||
|
||||
newBody.add quote do:
|
||||
return ok($cast[uint](`ctxIdent`))
|
||||
|
||||
@ -3,6 +3,19 @@
|
||||
import std/macros
|
||||
import ../codegen/meta
|
||||
|
||||
proc buildLibReadyGuard*(
|
||||
ctxHandlerName, libTypeName: NimNode
|
||||
): NimNode {.compileTime.} =
|
||||
## Rejects a request that reached the FFI thread with no library constructed.
|
||||
## Only for `ref` types: an `object` fallback is a usable zero value callers may
|
||||
## rely on, a `ref` one is nil. Sits in the handler, behind any queued ctor, so
|
||||
## calling without awaiting the create callback still works.
|
||||
quote:
|
||||
when `libTypeName` is ref:
|
||||
if not `ctxHandlerName`[].libReady.load():
|
||||
return
|
||||
err("library is not initialized: the constructor failed or has not run yet")
|
||||
|
||||
const scalarPodTypeNames = [
|
||||
"int", "int8", "int16", "int32", "int64", "uint", "uint8", "uint16", "uint32",
|
||||
"uint64", "byte", "float", "float32", "float64", "bool",
|
||||
@ -62,6 +75,9 @@ proc buildScalarPath*(
|
||||
let `reqIdent` = cast[ptr FFIThreadRequest](request)
|
||||
let `ctxHandlerName` = cast[`ctxType`](reqHandler)
|
||||
|
||||
# ctxType is `ptr FFIContext[LibType]`; the guard needs the library type.
|
||||
handlerBody.add(buildLibReadyGuard(ctxHandlerName, ctxType[0][1]))
|
||||
|
||||
let helperCall = newTree(nnkCall, userProcName)
|
||||
let ctxMyLib = newDotExpr(newTree(nnkDerefExpr, ctxHandlerName), ident("myLib"))
|
||||
helperCall.add(newTree(nnkDerefExpr, ctxMyLib))
|
||||
|
||||
@ -83,11 +83,11 @@ suite "{.ffi.} call after a failed constructor":
|
||||
check not ctx.isNil()
|
||||
check s.retCode.load() == int(RET_ERR)
|
||||
# `myLib` is non-nil even here: the FFI thread points it at a default-valued
|
||||
# fallback before dispatching. `myLibOwned` is what says the ctor stored a real
|
||||
# fallback before dispatching. `libReady` is what says the ctor stored a real
|
||||
# library.
|
||||
check not ctx[].myLib.isNil() # the fallback, not a constructed library
|
||||
check ctx[].myLib[].isNil() # ...and for a `ref` lib that fallback is nil
|
||||
check not ctx[].myLibOwned
|
||||
check not ctx[].libReady.load()
|
||||
|
||||
# The synchronous return only reports that the request was accepted; the
|
||||
# rejection itself comes back through the callback.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user