From 20fe91ae45b4113d0f06e6da4ff40afd28ef3812 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz <8129788+gmelodie@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:46:26 -0300 Subject: [PATCH] feat(c): non-scalar CBOR-free fast path (#137) --- CHANGELOG.md | 15 ++ ffi/codegen/c.nim | 2 +- ffi/ffi_thread_request.nim | 29 ++-- ffi/internal/c_macro_helpers.nim | 110 ++++++++------ ffi/internal/c_wire.nim | 36 ++++- ffi/internal/ffi_macro.nim | 163 ++++++++++++++------ ffi/internal/ffi_scalar.nim | 2 +- tests/unit/test_c_abi_dispatch.nim | 230 +++++++++++++++++++++++++++++ 8 files changed, 474 insertions(+), 113 deletions(-) create mode 100644 tests/unit/test_c_abi_dispatch.nim diff --git a/CHANGELOG.md b/CHANGELOG.md index 1508470..a4f9c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ All notable changes to this project are documented in this file. ## [Unreleased] ### Changed +- **`abi = c` non-scalar procs no longer marshal through CBOR.** The foreign + surface is unchanged — the generated headers and exported symbols are + byte-identical — but the hop between the caller and the FFI thread now carries + the packed `_CWire` struct itself instead of CBOR-encoding it and decoding it + back. A request is packed into a `malloc`'d owned copy on the calling thread + and unpacked (then freed) on the FFI thread; an object reply rides back as its + `_CWire` image and a `string` reply as raw UTF-8, so the round trip through + `cborEncodeShared`/`cborDecodePtr` is gone from both directions. Only the + scalar fast path was CBOR-free before + ([#131](https://github.com/logos-messaging/nim-ffi/issues/131)). +- `_CWire` seq/Option payload buffers are allocated with libc `malloc` + (`cwireAllocBuf`) rather than `allocShared`, so a wire packed on the calling + thread can be freed on the FFI thread — the cross-thread ownership the + CBOR-free request path relies on, and consistent with the libc-backed request + envelope. - The generated C `_ctx_destroy()` now returns `int` instead of `void`, propagating the exported `_destroy()` status code (`NIMFFI_RET_OK` on success, `RET_ERR` on a null/invalid context or a failed context teardown) diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index aa41a94..e4a2b5b 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -1333,7 +1333,7 @@ proc emitAbiMethod( func abiScalarOkLines(m: FFIProcMeta, fnType: string): seq[string] = ## Trampoline RET_OK branch. A string return rides as its own UTF-8; every - ## other scalar is the 8-byte image `ffiScalarRetBytes` packs (ints + ## other scalar is the 8-byte image `ffiRawRetBytes` packs (ints ## sign-extended, floats widened to double, bool as 0/1). let rt = m.returnTypeName.strip() if rt == "string" or rt == "cstring": diff --git a/ffi/ffi_thread_request.nim b/ffi/ffi_thread_request.nim index 22a037a..ea85ad3 100644 --- a/ffi/ffi_thread_request.nim +++ b/ffi/ffi_thread_request.nim @@ -17,11 +17,13 @@ type FFIThreadRequest* = object callback*: FFICallBack userData*: pointer reqId*: cstring ## Req type name used to look up the handler. - data*: ptr UncheckedArray[byte] ## Owned CBOR-encoded request payload. + data*: ptr UncheckedArray[byte] + ## Owned request payload: CBOR-encoded, or a packed `_CWire` struct on the + ## `abi = c` path. Nil on the scalar fast path. dataLen*: int - isScalar*: bool - ## Scalar fast path: args rode inline in `scalarArgs`, so a 0-length return - ## is a real empty string, not a CBOR "no value". + rawReply*: bool + ## CBOR-free request (scalar fast path or `abi = c`): the reply is raw bytes, + ## so a 0-length one is a real empty string, not a CBOR "no value". scalarArgs*: array[MaxScalarArgs, uint64] ## Inlined scalar args (no per-call c_malloc); a plain array keeps ## `deleteRequest` unaliased. @@ -63,7 +65,7 @@ proc allocBaseRequest( ret[].reqId = reqId.alloc() ret[].data = nil ret[].dataLen = 0 - ret[].isScalar = false + ret[].rawReply = false ret[].next = nil ret[].responded = false return ret @@ -121,11 +123,14 @@ proc initFromOwnedShared*( reqId: cstring, data: ptr UncheckedArray[byte], dataLen: int, + rawReply: bool = false, ): ptr type T = ## Adopts an already-c_malloc'd buffer (no copy); `deleteRequest` c_frees it. - ## Pass `(nil, 0)` for an empty payload. + ## Pass `(nil, 0)` for an empty payload. Set `rawReply` when the handler answers + ## with raw (non-CBOR) bytes, so an empty reply reads as a real empty value. var ret = allocBaseRequest(callback, userData, reqId) adoptOwnedSharedPayload(ret, data, dataLen) + ret[].rawReply = rawReply return ret proc initScalar*( @@ -140,14 +145,14 @@ proc initScalar*( "initScalar: " & $args.len & " scalar args exceed MaxScalarArgs (" & $MaxScalarArgs & ")" var ret = allocBaseRequest(callback, userData, reqId) - ret[].isScalar = true + ret[].rawReply = true for i in 0 ..< args.len: ret[].scalarArgs[i] = args[i] ret -func ffiScalarRetBytes*[T](x: T): seq[byte] = - ## Scalar handler result as raw bytes, no CBOR: string/cstring ride as UTF-8, - ## other scalars as the 8-byte native image of `ffiPackScalar(x)`. +func ffiRawRetBytes*[T](x: T): seq[byte] = + ## CBOR-free handler result as raw bytes: string/cstring ride as UTF-8, other + ## scalars as the 8-byte native image of `ffiPackScalar(x)`. when T is string: var b = newSeq[byte](x.len) if x.len > 0: @@ -195,8 +200,8 @@ proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest cast[csize_t](bytes.len), request[].userData, ) - elif request[].isScalar: - # Scalar 0-byte return is a real empty string, not CBOR "no value". + elif request[].rawReply: + # A CBOR-free 0-byte return is a real empty string, not CBOR "no value". var empty: byte request[].callback( RET_OK, cast[ptr cchar](addr empty), 0.csize_t, request[].userData diff --git a/ffi/internal/c_macro_helpers.nim b/ffi/internal/c_macro_helpers.nim index b51868e..59ed6e6 100644 --- a/ffi/internal/c_macro_helpers.nim +++ b/ffi/internal/c_macro_helpers.nim @@ -22,7 +22,7 @@ proc markCWireEmitted(typeName: string) {.compileTime.} = if not isCWireEmitted(typeName): emittedCWireTypes.add(typeName) -proc cwireTypeName(userTypeName: string): string = +proc cwireTypeName*(userTypeName: string): string = userTypeName & "_CWire" proc seqItemsField(obj, field: NimNode): NimNode = @@ -31,7 +31,7 @@ proc seqItemsField(obj, field: NimNode): NimNode = proc seqLenField(obj, field: NimNode): NimNode = newDotExpr(obj, ident($field & cwireLenSuffix)) -proc isStringType(t: NimNode): bool = +proc isStringType*(t: NimNode): bool = t.kind == nnkIdent and ($t == "string" or $t == "cstring") proc isBracketOf(t: NimNode, heads: openArray[string]): bool = @@ -304,12 +304,12 @@ proc emitSeqPack(dstObj, srcAccess, fieldNameIdent, userType: NimNode): NimNode `items` = nil `count` = 0 else: - `items` = cast[`bufType`](allocShared(sizeof(`wireElem`) * `srcAccess`.len())) + `items` = cast[`bufType`](cwireAllocBuf(sizeof(`wireElem`) * `srcAccess`.len())) `forLoop` `count` = `srcAccess`.len() proc emitOptionPack(dstAccess, srcAccess, userType: NimNode): NimNode = - ## Pack an Option into a `ptr`: some → `allocShared` box, none → nil. Payload + ## Pack an Option into a `ptr`: some → `cwireAllocBuf` box, none → nil. Payload ## read into a local once so a composite inner type isn't re-`get()` per element. let innerType = userType[1] let wireInner = wireValueType(innerType) @@ -318,7 +318,7 @@ proc emitOptionPack(dstAccess, srcAccess, userType: NimNode): NimNode = let elemPack = emitElemPack(nnkBracketExpr.newTree(dstAccess), innerVal, innerType) quote: if `srcAccess`.isSome(): - `dstAccess` = cast[`bufType`](allocShared(sizeof(`wireInner`))) + `dstAccess` = cast[`bufType`](cwireAllocBuf(sizeof(`wireInner`))) let `innerVal` = `srcAccess`.get() `elemPack` else: @@ -389,7 +389,7 @@ proc emitSeqFree(dstObj, fieldNameIdent, userType: NimNode): NimNode = quote: if not `items`.isNil(): `freeLoop` - deallocShared(`items`) + cwireFreeBuf(`items`) `items` = nil `count` = 0 @@ -400,7 +400,7 @@ proc emitOptionFree(dstAccess, userType: NimNode): NimNode = quote: if not `dstAccess`.isNil(): `freeInner` - deallocShared(`dstAccess`) + cwireFreeBuf(`dstAccess`) `dstAccess` = nil proc emitFreeStmt(dstObj, fieldNameIdent, userType: NimNode): seq[NimNode] = @@ -549,6 +549,9 @@ type paramNames: seq[string] ## envelope field names (the extra params) paramTypes: seq[NimNode] ## envelope field types respType: NimNode ## method result T; empty for a ctor + handler: NimNode + ## FFI-thread handler, deferred here so it lands after the `_CWire` + ## companions it packs/unpacks through. var cAbiSpecs {.compileTime.}: seq[CAbiSpec] @@ -563,7 +566,7 @@ proc registerCAbiMethod*( libType, envelope: NimNode, paramNames: seq[string], paramTypes: seq[NimNode], - respType: NimNode, + respType, handler: NimNode, ) {.compileTime.} = ## Record an `abi = c` method for `flushCAbiDispatch`. Nodes are `copyNimTree` ## frozen: reusing the Req section's originals (bound to `nnkSym`) would ICE. @@ -576,6 +579,7 @@ proc registerCAbiMethod*( paramNames: paramNames, paramTypes: copyTypes(paramTypes), respType: respType.copyNimTree(), + handler: handler.copyNimTree(), ) ) @@ -584,6 +588,7 @@ proc registerCAbiCtor*( libType, envelope: NimNode, paramNames: seq[string], paramTypes: seq[NimNode], + handler: NimNode, ) {.compileTime.} = ## Record an `abi = c` ctor for `flushCAbiDispatch`; see `registerCAbiMethod` ## for why nodes are `copyNimTree` frozen. @@ -596,6 +601,7 @@ proc registerCAbiCtor*( paramNames: paramNames, paramTypes: copyTypes(paramTypes), respType: newEmptyNode(), + handler: handler.copyNimTree(), ) ) @@ -640,15 +646,16 @@ proc replyTrampProc(trampName, body: NimNode): NimNode = pragmas = cdeclReplyPragma(), ) -proc objectTrampBody(boxName, respType, respWire: NimNode): NimNode = - ## Reply trampoline for an object return: decode, `cwirePack` into `_CWire`, - ## hand a pointer to the caller, release. `reply` is nil only on error. +proc objectTrampBody(boxName, respWire: NimNode): NimNode = + ## Reply trampoline for an object return: the payload is already the packed + ## `_CWire` image, so hand its address straight to the caller and release the + ## buffers it owns. `reply` is nil only on error. quote: let box = cast[ptr `boxName`](ud) if box.isNil(): return if ret == RET_STALE_WARN: - # Non-terminal progress signal: keep the box, don't decode. + # Non-terminal progress signal: keep the box, don't read the payload. return defer: freeBox(box) @@ -661,53 +668,45 @@ proc objectTrampBody(boxName, respType, respWire: NimNode): NimNode = copyMem(addr em[0], msg, int(len)) box.fn(ret, nil, em.cstring, box.ud) return - let decoded = - cborDecodePtr(cast[ptr UncheckedArray[byte]](msg), int(len), `respType`) - if decoded.isErr(): - box.fn(RET_ERR, nil, decoded.error.cstring, box.ud) - else: - var wire: `respWire` - cwirePack(wire, decoded.get()) - box.fn(RET_OK, addr wire, "".cstring, box.ud) - cwireFree(wire) + if msg.isNil() or int(len) != sizeof(`respWire`): + box.fn(RET_ERR, nil, "abi = c reply: unexpected payload size".cstring, box.ud) + return + var wire = cast[ptr `respWire`](msg)[] + box.fn(RET_OK, addr wire, "".cstring, box.ud) + cwireFree(wire) except CatchableError as e: box.fn(RET_ERR, nil, e.msg.cstring, box.ud) proc stringTrampBody(boxName: NimNode): NimNode = - ## Reply trampoline for a `string` return (and the ctor's address string): - ## decode and hand the caller a NUL-terminated `cstring`. Reply/error strings - ## are always non-nil empty on the paths they don't apply to (no nil deref). + ## Reply trampoline for a `string` return (and the ctor's address string): the + ## payload is raw length-delimited UTF-8, so copy it into a NUL-terminated + ## `cstring`. Whichever of reply/error is unused rides as empty, safe to deref. quote: let box = cast[ptr `boxName`](ud) if box.isNil(): return if ret == RET_STALE_WARN: - # Non-terminal progress signal: keep the box, don't decode. + # Non-terminal progress signal: keep the box, don't read the payload. return defer: freeBox(box) if box.fn.isNil(): return try: + var payload = newString(int(len)) + if int(len) > 0 and not msg.isNil(): + copyMem(addr payload[0], msg, int(len)) if ret != RET_OK: - var em = newString(int(len)) - if int(len) > 0: - copyMem(addr em[0], msg, int(len)) - box.fn(ret, "".cstring, em.cstring, box.ud) + box.fn(ret, "".cstring, payload.cstring, box.ud) return - let decoded = cborDecodePtr(cast[ptr UncheckedArray[byte]](msg), int(len), string) - if decoded.isErr(): - box.fn(RET_ERR, "".cstring, decoded.error.cstring, box.ud) - else: - let replyStr = decoded.get() - box.fn(RET_OK, replyStr.cstring, "".cstring, box.ud) + box.fn(RET_OK, payload.cstring, "".cstring, box.ud) except CatchableError as e: box.fn(RET_ERR, "".cstring, e.msg.cstring, box.ud) proc exportedMethodProc( spec: CAbiSpec, boxName, envWire, trampName, poolIdent, cbType: NimNode ): NimNode = - # No `foreignThreadGc`: `cwireUnpack`/`cborEncodeShared` alloc on the calling thread (already GC-registered); wrapping would free its live ORC heap. + # 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)) @@ -723,16 +722,20 @@ proc exportedMethodProc( if not `poolIdent`.isValidCtx(cast[pointer](ctx)): onReply(RET_ERR, `emptyReply`, "ctx is not a valid FFI context".cstring, userData) return RET_ERR - var reqObj: `envName` = cwireUnpack(req[]) - let enc = cborEncodeShared(reqObj) - let reqBuf = enc.data - let reqBufLen = enc.len + 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, reqBufLen + `trampName`, box, typeStr.cstring, reqBuf, sizeof(`envWire`), rawReply = true ) let sendRes = try: @@ -740,6 +743,10 @@ proc exportedMethodProc( except Exception as e: Result[void, string].err("sendRequestToFFIThread exception: " & e.msg) if sendRes.isErr(): + # A rejected send already `deleteRequest`ed the struct copy, which frees only + # the struct itself; `ownedWire` still aliases its field buffers, so free them + # here — on success the FFI thread's unpack does it instead. + cwireFree(ownedWire) onReply(RET_ERR, `emptyReply`, sendRes.error.cstring, userData) return RET_ERR return RET_OK @@ -784,16 +791,21 @@ proc exportedCtorProc( ) return nil let ctx = ctxRes.get() - var reqObj: `envName` = cwireUnpack(req[]) - let enc = cborEncodeShared(reqObj) - let reqBuf = enc.data - let reqBufLen = enc.len + var ownedWire: `envWire` + cwirePack(ownedWire, cwireUnpack(req[])) + let ownedCopy = cwireOwnedCopy(ownedWire) + if ownedCopy.isNil(): + cwireFree(ownedWire) + if not onCreated.isNil(): + onCreated(RET_ERR, "".cstring, "out of memory".cstring, userData) + return nil + let reqBuf = cast[ptr UncheckedArray[byte]](ownedCopy) let box = cast[ptr `boxName`](allocBox(sizeof(`boxName`))) box.fn = onCreated box.ud = userData let typeStr = $`envName` let reqPtr = FFIThreadRequest.initFromOwnedShared( - `trampName`, box, typeStr.cstring, reqBuf, reqBufLen + `trampName`, box, typeStr.cstring, reqBuf, sizeof(`envWire`), rawReply = true ) let sendRes = try: @@ -801,6 +813,9 @@ proc exportedCtorProc( 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) if not onCreated.isNil(): onCreated(RET_ERR, "".cstring, sendRes.error.cstring, userData) return nil @@ -848,6 +863,7 @@ proc flushCAbiDispatch*(): NimNode {.compileTime.} = for spec in cAbiSpecs: let envName = spec.envelope ensureCWireForFields(sink, $envName, spec.paramNames, spec.paramTypes) + sink.add(spec.handler) let envWire = ident(cwireTypeName($envName)) let boxName = ident($envName & "CBox") let trampName = ident($envName & "CReply") @@ -871,7 +887,7 @@ proc flushCAbiDispatch*(): NimNode {.compileTime.} = let respWire = ident(cwireTypeName($rt)) let cbType = cAbiCbType(nnkPtrTy.newTree(respWire)) sink.add(boxTypeDef(boxName, cbType)) - sink.add(replyTrampProc(trampName, objectTrampBody(boxName, rt, respWire))) + sink.add(replyTrampProc(trampName, objectTrampBody(boxName, respWire))) sink.add( exportedMethodProc(spec, boxName, envWire, trampName, poolIdent, cbType) ) diff --git a/ffi/internal/c_wire.nim b/ffi/internal/c_wire.nim index 3d60fed..bfadc78 100644 --- a/ffi/internal/c_wire.nim +++ b/ffi/internal/c_wire.nim @@ -1,14 +1,44 @@ -## Runtime cstring alloc/free for the macro-generated `*_CWire` types. +## Memory helpers for the macro-generated `*_CWire` types — the flat C-ABI mirror +## of a Nim object, where strings and seq/Option payloads live in separate buffers +## the struct only points at. These procs allocate and free those buffers, and copy +## the struct across the hop to the FFI thread. import ../alloc +proc cwireAllocBuf*(size: int): pointer = + ## Buffer for a wire seq/Option payload. libc `malloc` rather than `allocShared` + ## so one thread can allocate and a different thread can free (see ../alloc). + alloc.allocBox(size) + +proc cwireFreeBuf*(p: pointer) = + ## Frees a `cwireAllocBuf` buffer; does nothing if `p` is nil. + alloc.freeBox(p) + proc cwireAllocStr*(s: string): cstring {.inline.} = - ## NUL-terminated `malloc` copy of `s`; pair with `cwireFreeStr`. + ## NUL-terminated copy of `s` for a wire string field; free with `cwireFreeStr`. alloc.alloc(s) proc cwireFreeStr*(s: var cstring) {.inline.} = - ## Idempotent free; reset to `nil` so a repeated call can't double-free. + ## Frees a wire string field and nils it, so freeing twice is harmless. if s.isNil(): return alloc.dealloc(s) s = nil + +func cwireStructBytes*[W](wire: W): seq[byte] = + ## The struct's raw bytes, to hand a reply back over the FFI-thread hop. Copies + ## the pointers, not what they point at, so `wire`'s buffers must stay alive + ## until the receiver `cwireFree`s them. + var b = newSeq[byte](sizeof(W)) + copyMem(addr b[0], unsafeAddr wire, sizeof(W)) + b + +proc cwireOwnedCopy*[W](wire: W): ptr W = + ## The same shallow copy, into `malloc` memory the FFI thread adopts and frees; + ## nil if the allocation fails. `copyMem` because assigning into raw `malloc` + ## bytes would run ORC's copy hooks over uninitialised memory. + let p = cast[ptr W](alloc.allocBox(sizeof(W))) + if p.isNil(): + return nil + copyMem(p, unsafeAddr wire, sizeof(W)) + return p diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index 34fd413..e46fda2 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -411,8 +411,37 @@ proc buildFFINewReqProc(reqTypeName, body: NimNode): NimNode = echo newReqProc.repr return newReqProc -proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode = - ## FFI-thread processor: decodes the CBOR Req, unpacks fields, runs user body. +proc reqDecodePreamble( + reqTypeName, reqIdent, decodedIdent: NimNode, abi: ABIFormat +): NimNode = + ## Materialise the typed Req from the request payload. `abi = c` unpacks the + ## packed `_CWire` struct the caller thread handed over and frees it here (the + ## unpack deep-copies into Nim memory); the envelope buffer itself goes with + ## `deleteRequest`. Otherwise the payload is CBOR. + if abi != ABIFormat.C: + return quote: + let `reqIdent`: ptr FFIThreadRequest = cast[ptr FFIThreadRequest](request) + let `decodedIdent` = cborDecodePtr( + cast[ptr UncheckedArray[byte]](`reqIdent`[].data), + `reqIdent`[].dataLen, + `reqTypeName`, + ).valueOr: + return err("CBOR decode failed for " & $T & ": " & $error) + + let wireType = ident(cwireTypeName($reqTypeName)) + let wirePtr = genSym(nskLet, "wireReq") + return quote: + let `reqIdent`: ptr FFIThreadRequest = cast[ptr FFIThreadRequest](request) + if `reqIdent`[].data.isNil() or `reqIdent`[].dataLen != sizeof(`wireType`): + return err("abi = c: unexpected request payload size for " & $T) + let `wirePtr` = cast[ptr `wireType`](`reqIdent`[].data) + let `decodedIdent` = cwireUnpack(`wirePtr`[]) + cwireFree(`wirePtr`[]) + +proc buildProcessFFIRequestProc( + reqTypeName, reqHandler, body: NimNode, abi: ABIFormat +): NimNode = + ## FFI-thread processor: materialises the Req, unpacks fields, runs user body. if reqHandler.kind != nnkExprColonExpr: error( "Second argument must be a typed parameter, e.g., waku: ptr Waku. Found: " & @@ -449,14 +478,7 @@ proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode let reqIdent = genSym(nskLet, "ffiReq") let decodedIdent = genSym(nskLet, "decoded") - newBody.add quote do: - let `reqIdent`: ptr FFIThreadRequest = cast[ptr FFIThreadRequest](request) - let `decodedIdent` = cborDecodePtr( - cast[ptr UncheckedArray[byte]](`reqIdent`[].data), - `reqIdent`[].dataLen, - `reqTypeName`, - ).valueOr: - return err("CBOR decode failed for " & $T & ": " & $error) + newBody.add reqDecodePreamble(reqTypeName, reqIdent, decodedIdent, abi) for p in procParams[1 ..^ 1]: if isHandleType(p[1]): @@ -482,9 +504,44 @@ proc buildProcessFFIRequestProc(reqTypeName, reqHandler, body: NimNode): NimNode echo processProc.repr return processProc -proc addNewRequestToRegistry(reqTypeName, reqHandler: NimNode): NimNode = - ## Dispatcher the FFI thread calls: runs processFFIRequest and cborEncodes the - ## typed T value into the seq[byte] payload. +proc replyEncode( + typedResIdent, handlerCtxIdent, respType: NimNode, abi: ABIFormat +): NimNode = + ## Lower the handler's typed value into the `seq[byte]` reply payload. `abi = c` + ## rides raw — a `string` as its own UTF-8, an object as the native image of its + ## packed `_CWire`, whose buffers the reply trampoline frees. + if abi == ABIFormat.C: + if isStringType(respType): + return quote: + return ok(ffiRawRetBytes(`typedResIdent`.value)) + let wireType = ident(cwireTypeName($respType)) + let wireIdent = genSym(nskVar, "replyWire") + return quote: + var `wireIdent`: `wireType` + cwirePack(`wireIdent`, `typedResIdent`.value) + return ok(cwireStructBytes(`wireIdent`)) + + return quote: + when typeof(`typedResIdent`.value) is seq[byte]: + return ok(`typedResIdent`.value) + elif typeof(`typedResIdent`.value) is void: + return ok(newSeq[byte]()) + elif typeof(`typedResIdent`.value) is FFIHandleRoot: + return ok( + encodeHandle( + `handlerCtxIdent`[].handles.register( + `typedResIdent`.value, $typeof(`typedResIdent`.value) + ) + ) + ) + else: + return ok(cborEncode(`typedResIdent`.value)) + +proc addNewRequestToRegistry( + reqTypeName, reqHandler, respType: NimNode, abi: ABIFormat +): NimNode = + ## Dispatcher the FFI thread calls: runs processFFIRequest and lowers the typed + ## T value into the seq[byte] payload. let returnType = nnkBracketExpr.newTree( ident("Future"), nnkBracketExpr.newTree( @@ -516,20 +573,8 @@ proc addNewRequestToRegistry(reqTypeName, reqHandler: NimNode): NimNode = let `typedResIdent` = await `callExpr` if `typedResIdent`.isErr: return err(`typedResIdent`.error) - when typeof(`typedResIdent`.value) is seq[byte]: - return ok(`typedResIdent`.value) - elif typeof(`typedResIdent`.value) is void: - return ok(newSeq[byte]()) - elif typeof(`typedResIdent`.value) is FFIHandleRoot: - return ok( - encodeHandle( - `handlerCtxIdent`[].handles.register( - `typedResIdent`.value, $typeof(`typedResIdent`.value) - ) - ) - ) - else: - return ok(cborEncode(`typedResIdent`.value)) + + newBody.add replyEncode(typedResIdent, handlerCtxIdent, respType, abi) let asyncProc = newProc( name = newEmptyNode(), @@ -556,8 +601,10 @@ macro registerReqFFI*(reqTypeName, reqHandler, body: untyped): untyped = ## Future[Result[string, string]] {.async.}. let typeDef = buildRequestType(reqTypeName, body) let ffiNewReqProc = buildFFINewReqProc(reqTypeName, body) - let processProc = buildProcessFFIRequestProc(reqTypeName, reqHandler, body) - let addNewReqToReg = addNewRequestToRegistry(reqTypeName, reqHandler) + let processProc = + buildProcessFFIRequestProc(reqTypeName, reqHandler, body, ABIFormat.Cbor) + let addNewReqToReg = + addNewRequestToRegistry(reqTypeName, reqHandler, newEmptyNode(), ABIFormat.Cbor) let stmts = newStmtList(typeDef, ffiNewReqProc, processProc, addNewReqToReg) when defined(ffiDumpMacros): @@ -1011,12 +1058,17 @@ macro ffi*(args: varargs[untyped]): untyped = ffiProcRegistry.add(procMeta) if abiFormat == ABIFormat.C: - # The `abi = c` wrapper + reply trampoline are emitted at genBindings() time (flushCAbiDispatch); the CBOR `ffiProc` is not. + # The handler unpacks through the `_CWire` companions, which only exist once every `{.ffi.}` type has been seen, so it (with the wrapper + reply trampoline) is emitted at genBindings() time (flushCAbiDispatch). The Req type stays here for the companion to name. The CBOR `ffiProc`/`ffiNewReq` aren't emitted at all. + let handlerParam = nnkExprColonExpr.newTree(ctxHandlerName, ptrFFICtx) + let handler = newStmtList( + buildProcessFFIRequestProc(reqTypeName, handlerParam, lambdaNode, ABIFormat.C), + addNewRequestToRegistry(reqTypeName, handlerParam, resultRetType, ABIFormat.C), + ) registerCAbiMethod( cExportName, libTypeName, reqTypeName, extraParamNames, extraParamTypes, - resultRetType, + resultRetType, handler, ) - return newStmtList(helperProc, registerReq) + return newStmtList(helperProc, buildRequestType(reqTypeName, lambdaNode)) return newStmtList(helperProc, registerReq, ffiProc) @@ -1144,8 +1196,9 @@ proc buildCtorProcessFFIRequestProc( paramNames: seq[string], paramTypes: seq[NimNode], libTypeName: NimNode, + abi: ABIFormat, ): NimNode = - ## Decodes the Req, runs the user body, stores the library value in ctx.myLib. + ## Materialises the Req, runs the user body, stores the library value in ctx.myLib. let returnType = nnkBracketExpr.newTree( ident("Future"), nnkBracketExpr.newTree(ident("Result"), ident("string"), ident("string")), @@ -1168,14 +1221,7 @@ proc buildCtorProcessFFIRequestProc( let ctxIdent = ident("ctx") let decodedIdent = ident("decoded") - newBody.add quote do: - let `reqIdent` = cast[ptr FFIThreadRequest](request) - let `decodedIdent` = cborDecodePtr( - cast[ptr UncheckedArray[byte]](`reqIdent`[].data), - `reqIdent`[].dataLen, - `reqTypeName`, - ).valueOr: - return err("CBOR decode failed for " & $T & ": " & $error) + newBody.add reqDecodePreamble(reqTypeName, reqIdent, decodedIdent, abi) for i in 0 ..< paramNames.len: newBody.add unpackReqField(ident(paramNames[i]), paramTypes[i], decodedIdent) @@ -1209,9 +1255,12 @@ proc buildCtorProcessFFIRequestProc( echo processProc.repr return processProc -proc addCtorRequestToRegistry(reqTypeName, libTypeName: NimNode): NimNode = +proc addCtorRequestToRegistry( + reqTypeName, libTypeName: NimNode, abi: ABIFormat +): NimNode = ## Wraps the ctor processFFIRequest result in a seq[byte] dispatcher; the ctor - ## returns the ctx address as a decimal string, CBOR-encoded for the foreign side. + ## returns the ctx address as a decimal string — raw UTF-8 under `abi = c`, + ## CBOR-encoded otherwise. let ctxType = nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), libTypeName)) @@ -1231,12 +1280,21 @@ proc addCtorRequestToRegistry(reqTypeName, libTypeName: NimNode): NimNode = ) let resIdent = genSym(nskLet, "ctorRes") + let encodeRet = + if abi == ABIFormat.C: + quote: + return ok(ffiRawRetBytes(`resIdent`.value)) + else: + quote: + return ok(cborEncode(`resIdent`.value)) + var newBody = newStmtList() newBody.add quote do: let `resIdent` = await `callExpr` if `resIdent`.isErr: return err(`resIdent`.error) - return ok(cborEncode(`resIdent`.value)) + + newBody.add encodeRet let asyncProc = newProc( name = newEmptyNode(), @@ -1318,9 +1376,9 @@ macro ffiCtor*(args: varargs[untyped]): untyped = let helperProc = buildCtorBodyProc(userProcName, paramNames, paramTypes, libTypeName, bodyNode) let processProc = buildCtorProcessFFIRequestProc( - reqTypeName, userProcName, paramNames, paramTypes, libTypeName + reqTypeName, userProcName, paramNames, paramTypes, libTypeName, abiFormat ) - let addToReg = addCtorRequestToRegistry(reqTypeName, libTypeName) + let addToReg = addCtorRequestToRegistry(reqTypeName, libTypeName, abiFormat) # C-exported proc: (reqCbor, reqCborLen, callback, userData) -> pointer var exportedParams = newSeq[NimNode]() @@ -1432,9 +1490,16 @@ macro ffiCtor*(args: varargs[untyped]): untyped = let stmts = if abiFormat == ABIFormat.C: - # The `abi = c` wrapper is emitted at genBindings() time; CBOR `ffiProc` isn't. - registerCAbiCtor(cExportName, libTypeName, reqTypeName, paramNames, paramTypes) - newStmtList(typeDef, ffiNewReqProc, helperProc, processProc, addToReg, poolDecl) + # The `abi = c` handler + wrapper are emitted at genBindings() time (the handler unpacks through the `_CWire` companions); the CBOR `ffiProc`/`ffiNewReq` aren't emitted at all. + registerCAbiCtor( + cExportName, + libTypeName, + reqTypeName, + paramNames, + paramTypes, + newStmtList(processProc, addToReg), + ) + newStmtList(typeDef, helperProc, poolDecl) else: newStmtList( typeDef, ffiNewReqProc, helperProc, processProc, addToReg, poolDecl, ffiProc diff --git a/ffi/internal/ffi_scalar.nim b/ffi/internal/ffi_scalar.nim index 3fdde24..2086f20 100644 --- a/ffi/internal/ffi_scalar.nim +++ b/ffi/internal/ffi_scalar.nim @@ -79,7 +79,7 @@ proc buildScalarPath*( handlerBody.add quote do: let `retValIdent` = (await `helperCall`).valueOr: return err(error) - return ok(ffiScalarRetBytes(`retValIdent`)) + return ok(ffiRawRetBytes(`retValIdent`)) let seqByteResult = nnkBracketExpr.newTree( ident("Future"), diff --git a/tests/unit/test_c_abi_dispatch.nim b/tests/unit/test_c_abi_dispatch.nim new file mode 100644 index 0000000..a3bcb02 --- /dev/null +++ b/tests/unit/test_c_abi_dispatch.nim @@ -0,0 +1,230 @@ +## Exercises the CBOR-free `abi = c` non-scalar path through a real FFI thread: +## the request rides as a packed `_CWire` struct and the reply comes back as the +## response's `_CWire` image. Covers `seq`/`Option` fields, whose payload buffers +## are packed on the calling thread and freed on the FFI thread. + +import std/[locks, options, strutils] +import unittest2 +import results +import ffi + +type WireLib = object + tag: string + +# Stub the dylib NimMain importc that declareLibrary emits (links as an exe). +{.emit: "void libwirefastNimMain(void) {}".} + +declareLibrary("wirefast", WireLib, defaultABIFormat = "c") + +type WireConfig {.ffi.} = object + tag: string + +type BulkRequest {.ffi.} = object + items: seq[string] + note: Option[string] + +type BulkResponse {.ffi.} = object + joined: string + count: int + echoed: seq[string] + +proc wirefast_create*(cfg: WireConfig): Future[Result[WireLib, string]] {.ffiCtor.} = + return ok(WireLib(tag: cfg.tag)) + +proc wirefast_bulk*( + lib: WireLib, req: BulkRequest +): Future[Result[BulkResponse, string]] {.ffi.} = + let note = req.note.get("none") + return ok( + BulkResponse( + joined: lib.tag & ":" & req.items.join(",") & "/" & note, + count: req.items.len, + echoed: req.items, + ) + ) + +proc wirefast_greet*( + lib: WireLib, req: BulkRequest +): Future[Result[string, string]] {.ffi.} = + ## String return: rides back as raw UTF-8, not CBOR. + if req.items.len == 0: + return err("no items") + return ok(lib.tag & " greets " & req.items[0]) + +genBindings() + +type ReplyData = object + ## One latch per call; `text` doubles as the ctor's address string. + lock: Lock + cond: Cond + called: bool + retCode: cint + text: string + errMsg: string + count: int + echoed: seq[string] + +proc initReplyData(d: var ReplyData) = + d.lock.initLock() + d.cond.initCond() + +proc deinitReplyData(d: var ReplyData) = + d.cond.deinitCond() + d.lock.deinitLock() + +proc waitReply(d: var ReplyData) = + acquire(d.lock) + while not d.called: + wait(d.cond, d.lock) + release(d.lock) + +proc signalReply(d: ptr ReplyData, err: cint, errMsg: cstring) = + d[].retCode = err + if err != RET_OK and not errMsg.isNil(): + d[].errMsg = $errMsg + d[].called = true + signal(d[].cond) + release(d[].lock) + +proc onStringReply( + err: cint, reply: cstring, errMsg: cstring, ud: pointer +) {.cdecl, gcsafe, raises: [].} = + ## Shared by the ctor's address string and `greet`'s raw-UTF-8 return. + let d = cast[ptr ReplyData](ud) + acquire(d[].lock) + if err == RET_OK and not reply.isNil(): + d[].text = $reply + signalReply(d, err, errMsg) + +proc onBulkReply( + err: cint, reply: ptr BulkResponse_CWire, errMsg: cstring, ud: pointer +) {.cdecl, gcsafe, raises: [].} = + ## Reads the reply wire struct before the trampoline frees it. + let d = cast[ptr ReplyData](ud) + acquire(d[].lock) + if err == RET_OK and not reply.isNil(): + d[].text = $reply[].joined + d[].count = reply[].count + d[].echoed = @[] + for i in 0 ..< reply[].echoed_len: + d[].echoed.add($reply[].echoed_items[i]) + signalReply(d, err, errMsg) + +proc packedWire[W, R](_: typedesc[W], envelope: R): W = + ## The caller-owned request struct a foreign caller would hand the wrapper. + var wire: W + cwirePack(wire, envelope) + wire + +proc bulkReq(items: seq[string], note: Option[string]): BulkRequest = + BulkRequest(items: items, note: note) + +proc makeCtx(tag: string): ptr FFIContext[WireLib] = + var d: ReplyData + initReplyData(d) + defer: + deinitReplyData(d) + + var wire = packedWire( + WirefastCreateCtorReq_CWire, WirefastCreateCtorReq(cfg: WireConfig(tag: tag)) + ) + defer: + cwireFree(wire) + + doAssert not WirefastCreateCtorReqCAbiExport(addr wire, onStringReply, addr d).isNil() + waitReply(d) + doAssert d.retCode == RET_OK + cast[ptr FFIContext[WireLib]](cast[uint](parseBiggestUInt(d.text))) + +suite "abi = c non-scalar dispatch — CBOR-free wire transport": + test "seq + Option request round-trips into an object reply": + let ctx = makeCtx("bulk") + defer: + check WireLibFFIPool.destroyFFIContext(ctx).isOk() + + var d: ReplyData + initReplyData(d) + defer: + deinitReplyData(d) + + var req = packedWire( + WirefastBulkReq_CWire, WirefastBulkReq(req: bulkReq(@["a", "b", "c"], some("hi"))) + ) + defer: + cwireFree(req) + + check WirefastBulkReqCAbiExport(ctx, onBulkReply, addr d, addr req) == RET_OK + waitReply(d) + + check d.retCode == RET_OK + check d.text == "bulk:a,b,c/hi" + check d.count == 3 + check d.echoed == @["a", "b", "c"] + + test "none Option and empty seq survive the hop": + let ctx = makeCtx("empty") + defer: + check WireLibFFIPool.destroyFFIContext(ctx).isOk() + + var d: ReplyData + initReplyData(d) + defer: + deinitReplyData(d) + + var req = packedWire( + WirefastBulkReq_CWire, WirefastBulkReq(req: bulkReq(@[], none(string))) + ) + defer: + cwireFree(req) + + check WirefastBulkReqCAbiExport(ctx, onBulkReply, addr d, addr req) == RET_OK + waitReply(d) + + check d.retCode == RET_OK + check d.text == "empty:/none" + check d.count == 0 + check d.echoed.len == 0 + + test "string return rides back as raw UTF-8": + let ctx = makeCtx("greeter") + defer: + check WireLibFFIPool.destroyFFIContext(ctx).isOk() + + var d: ReplyData + initReplyData(d) + defer: + deinitReplyData(d) + + var req = packedWire( + WirefastGreetReq_CWire, WirefastGreetReq(req: bulkReq(@["world"], none(string))) + ) + defer: + cwireFree(req) + + check WirefastGreetReqCAbiExport(ctx, onStringReply, addr d, addr req) == RET_OK + waitReply(d) + + check d.retCode == RET_OK + check d.text == "greeter greets world" + + test "handler error surfaces as RET_ERR with the message": + let ctx = makeCtx("greeter") + defer: + check WireLibFFIPool.destroyFFIContext(ctx).isOk() + + var d: ReplyData + initReplyData(d) + defer: + deinitReplyData(d) + + var req = packedWire( + WirefastGreetReq_CWire, WirefastGreetReq(req: bulkReq(@[], none(string))) + ) + defer: + cwireFree(req) + + check WirefastGreetReqCAbiExport(ctx, onStringReply, addr d, addr req) == RET_OK + waitReply(d) + + check d.retCode == RET_ERR + check d.errMsg == "no items"