mirror of
https://github.com/logos-messaging/nim-ffi.git
synced 2026-07-31 12:03:11 +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, and
`myLibOwned` is what distinguishes that fallback from a real one. For an
`object` library the fallback is a usable zero value and callers legitimately
depend on it, but for a `ref` library it 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.
Guard on `myLibOwned` for `ref` library types only. 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.
Backport of the same fix on master, where the state had to be added rather than
reused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULz7Md52AF6PmqZeCmh8b7
This commit is contained in:
parent
75b0726924
commit
2e30f06757
11
CHANGELOG.md
11
CHANGELOG.md
@ -4,6 +4,17 @@ All notable changes to this project are documented in this file.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- A `{.ffi.}` call against a `ref` library type whose `{.ffiCtor.}` never stored
|
||||
a library (it failed, or none ran) no longer crashes. Without a constructed
|
||||
library the FFI thread points `myLib` at a default-valued fallback; for an
|
||||
`object` that is a usable zero value, but for a `ref` it is `nil`, so the user
|
||||
body faulted on its first field access. Such a request is now rejected with
|
||||
`library is not initialized: the constructor failed or has not run yet`
|
||||
through the callback. The check is emitted only for `ref` library types, and
|
||||
it runs on the FFI thread behind any queued constructor, so a host that issues
|
||||
a call without awaiting the create callback is unaffected.
|
||||
|
||||
### Changed
|
||||
- User event callbacks now run on a dedicated event thread fed by a
|
||||
bounded SPSC queue (default capacity 1024), so a slow listener can no
|
||||
|
||||
@ -886,6 +886,20 @@ macro ffi*(args: varargs[untyped]): untyped =
|
||||
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"
|
||||
)
|
||||
let retValIdent = ident("retVal")
|
||||
lambdaBody.add quote do:
|
||||
let `retValIdent` = (await `helperCall`).valueOr:
|
||||
|
||||
142
tests/unit/test_ctx_after_failed_ctor.nim
Normal file
142
tests/unit/test_ctx_after_failed_ctor.nim
Normal file
@ -0,0 +1,142 @@
|
||||
import std/[atomics, os]
|
||||
import unittest2
|
||||
import results
|
||||
import ffi
|
||||
|
||||
# A failing {.ffiCtor.} still hands the caller a live context — the ctor body
|
||||
# runs on the FFI thread, long after the C entry point returned the pointer.
|
||||
# `myLib` is not nil (the FFI thread points it at a default-valued fallback), but
|
||||
# for a `ref` library type that default IS nil, so a later {.ffi.} call used to
|
||||
# hand the user body a nil ref and crash on its first field access.
|
||||
|
||||
type FailedCtorLib = ref object
|
||||
marker: int
|
||||
|
||||
# Stub the importc NimMain declareLibrary emits (plain-exe link).
|
||||
{.emit: "void libfailedctorNimMain(void) {}".}
|
||||
|
||||
declareLibrary("failedctor", FailedCtorLib)
|
||||
|
||||
type FailedCtorConfig {.ffi.} = object
|
||||
shouldFail: bool
|
||||
|
||||
proc failedctor_create*(
|
||||
config: FailedCtorConfig
|
||||
): Future[Result[FailedCtorLib, string]] {.ffiCtor.} =
|
||||
if config.shouldFail:
|
||||
return err("ctor deliberately failed")
|
||||
return ok(FailedCtorLib(marker: 1))
|
||||
|
||||
# Both bodies touch a field, which is what faults on a nil ref receiver.
|
||||
proc failedctor_ping*(lib: FailedCtorLib): Future[Result[string, string]] {.ffi.} =
|
||||
return ok("pong:" & $lib.marker)
|
||||
|
||||
proc failedctor_echo*(
|
||||
lib: FailedCtorLib, note: string
|
||||
): Future[Result[string, string]] {.ffi.} =
|
||||
return ok(note & ":" & $lib.marker)
|
||||
|
||||
type CallbackState = object
|
||||
called: Atomic[bool]
|
||||
retCode: Atomic[int]
|
||||
|
||||
proc resetState(s: var CallbackState) =
|
||||
s.called.store(false)
|
||||
s.retCode.store(-1)
|
||||
|
||||
proc recordingCallback(
|
||||
retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer
|
||||
) {.cdecl, gcsafe, raises: [].} =
|
||||
let s = cast[ptr CallbackState](userData)
|
||||
s[].retCode.store(int(retCode))
|
||||
s[].called.store(true)
|
||||
|
||||
proc encodedPtr(bytes: var seq[byte]): ptr byte =
|
||||
if bytes.len == 0:
|
||||
nil
|
||||
else:
|
||||
cast[ptr byte](addr bytes[0])
|
||||
|
||||
proc waitCalled(s: var CallbackState): bool =
|
||||
var tries = 0
|
||||
while not s.called.load() and tries < 500:
|
||||
os.sleep(5)
|
||||
inc tries
|
||||
s.called.load()
|
||||
|
||||
proc createFailedCtx(s: var CallbackState): ptr FFIContext[FailedCtorLib] =
|
||||
## Drives the ctor down its error path and returns the still-live context.
|
||||
resetState(s)
|
||||
var cfg =
|
||||
cborEncode(FailedctorCreateCtorReq(config: FailedCtorConfig(shouldFail: true)))
|
||||
let ret =
|
||||
failedctor_create(encodedPtr(cfg), cfg.len.csize_t, recordingCallback, addr s)
|
||||
if ret.isNil():
|
||||
return nil
|
||||
discard waitCalled(s)
|
||||
cast[ptr FFIContext[FailedCtorLib]](ret)
|
||||
|
||||
suite "{.ffi.} call after a failed constructor":
|
||||
test "the failed ctor reports the error but leaves the context alive":
|
||||
var s: CallbackState
|
||||
let ctx = createFailedCtx(s)
|
||||
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
|
||||
# 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
|
||||
|
||||
# The synchronous return only reports that the request was accepted; the
|
||||
# rejection itself comes back through the callback.
|
||||
test "a no-arg call on an uninitialized library reports RET_ERR, no crash":
|
||||
var s: CallbackState
|
||||
let ctx = createFailedCtx(s)
|
||||
check not ctx.isNil()
|
||||
|
||||
resetState(s)
|
||||
var req = cborEncode(FailedctorPingReq())
|
||||
let ret =
|
||||
failedctor_ping(ctx, recordingCallback, addr s, encodedPtr(req), req.len.csize_t)
|
||||
check ret == RET_OK
|
||||
check waitCalled(s)
|
||||
check s.retCode.load() == int(RET_ERR)
|
||||
|
||||
test "an argument-taking call on an uninitialized library reports RET_ERR, no crash":
|
||||
var s: CallbackState
|
||||
let ctx = createFailedCtx(s)
|
||||
check not ctx.isNil()
|
||||
|
||||
resetState(s)
|
||||
var req = cborEncode(FailedctorEchoReq(note: "hello"))
|
||||
let ret =
|
||||
failedctor_echo(ctx, recordingCallback, addr s, encodedPtr(req), req.len.csize_t)
|
||||
check ret == RET_OK
|
||||
check waitCalled(s)
|
||||
check s.retCode.load() == int(RET_ERR)
|
||||
|
||||
# The guard runs on the FFI thread, behind the queued ctor, so it must not
|
||||
# penalise a host that fires a call without first awaiting the create callback.
|
||||
test "a call issued before the successful ctor callback still succeeds":
|
||||
var ctorState: CallbackState
|
||||
resetState(ctorState)
|
||||
var cfg =
|
||||
cborEncode(FailedctorCreateCtorReq(config: FailedCtorConfig(shouldFail: false)))
|
||||
let raw = failedctor_create(
|
||||
encodedPtr(cfg), cfg.len.csize_t, recordingCallback, addr ctorState
|
||||
)
|
||||
check not raw.isNil()
|
||||
let ctx = cast[ptr FFIContext[FailedCtorLib]](raw)
|
||||
|
||||
# Deliberately no wait: the request queues behind the in-flight ctor.
|
||||
var callState: CallbackState
|
||||
resetState(callState)
|
||||
var req = cborEncode(FailedctorPingReq())
|
||||
let ret = failedctor_ping(
|
||||
ctx, recordingCallback, addr callState, encodedPtr(req), req.len.csize_t
|
||||
)
|
||||
check ret == RET_OK
|
||||
check waitCalled(callState)
|
||||
check callState.retCode.load() == int(RET_OK)
|
||||
Loading…
x
Reference in New Issue
Block a user