From 2e30f067574b9b71c291ee90bfd64a2a05103f06 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Tue, 28 Jul 2026 02:44:09 +0200 Subject: [PATCH] fix: reject {.ffi.} calls on an unconstructed ref library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01ULz7Md52AF6PmqZeCmh8b7 --- CHANGELOG.md | 11 ++ ffi/internal/ffi_macro.nim | 14 +++ tests/unit/test_ctx_after_failed_ctor.nim | 142 ++++++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 tests/unit/test_ctx_after_failed_ctor.nim diff --git a/CHANGELOG.md b/CHANGELOG.md index 67f2948..078ee71 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index eaf18d2..6a68a60 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -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: diff --git a/tests/unit/test_ctx_after_failed_ctor.nim b/tests/unit/test_ctx_after_failed_ctor.nim new file mode 100644 index 0000000..d98cb40 --- /dev/null +++ b/tests/unit/test_ctx_after_failed_ctor.nim @@ -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)