From 007629c254e6d31858e704a2761d309031a98e12 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Fri, 5 Jun 2026 18:06:18 +0200 Subject: [PATCH] refactor(pool): rename slots to contexts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pool's fixed array holds FFIContext objects, so "contexts" names what it actually stores; "slot" emphasised array positions but the element type already conveys that. Swept the "slot" vocabulary to "context" across the pool, context, macro and test prose so the terminology is consistent with the field name. Also corrected a stale doc reference — pool.releaseSlot (no such proc on this branch) is now the real mechanism, ctx.unclaim(). Co-Authored-By: Claude Opus 4.8 --- ffi/ffi_context.nim | 34 +++++++++++++++++----------------- ffi/ffi_context_pool.nim | 30 +++++++++++++++--------------- ffi/internal/ffi_macro.nim | 2 +- tests/test_ffi_context.nim | 32 ++++++++++++++++---------------- 4 files changed, 49 insertions(+), 49 deletions(-) diff --git a/ffi/ffi_context.nim b/ffi/ffi_context.nim index f7a8a4a..41bd904 100644 --- a/ffi/ffi_context.nim +++ b/ffi/ffi_context.nim @@ -14,7 +14,7 @@ type FFICallbackState* = object type CtxLifecycle {.pure.} = enum ## Request-acceptance + recycle handshake for a pooled context, held as an - ## Atomic on FFIContext. ("Recycle" = drain the context and return its slot to + ## Atomic on FFIContext. ("Recycle" = drain the context and return its context to ## the pool for reuse, keeping the worker alive — unlike destroyFFIContext, which ## fully tears the threads down.) Invariants: ## * Requests are accepted ONLY in `Active`; the gate in sendRequestToFFIThread @@ -23,7 +23,7 @@ type CtxLifecycle {.pure.} = enum ## Active -> RecyclePending requestRecycle (caller, under `lock`) ## RecyclePending -> Recycling the FFI loop (one-shot compareExchange) ## Recycling -> Active markReacquired (caller, on reuse) - ## (initContextResources starts a slot in `Active`.) + ## (initContextResources starts a context in `Active`.) ## * The gate stays closed across BOTH RecyclePending and Recycling, so no ## request can dispatch onto a context being recycled or about to be reused. ## * Only the FFI loop makes the RecyclePending -> Recycling move, so the @@ -61,8 +61,8 @@ type FFIContext*[T] = object # RET_OK once drained, RET_ERR if it timed out. Set by requestRecycle. recycleUserData: pointer inUse: Atomic[bool] - # Whether the slot is claimed. createFFIContext claims it (false -> true); the - # recycle handler clears it once drained. On the slot so the owning thread can + # Whether the context is claimed. createFFIContext claims it (false -> true); the + # recycle handler clears it once drained. On the context so the owning thread can # release it without reaching into the pool. registeredRequests: ptr Table[cstring, FFIRequestProc] # Pointer to with the registered requests at compile time @@ -129,7 +129,7 @@ proc sendRequestToFFIThread*( ctx.lock.release() ## A recycle closes this gate (under the same lock), so a queued or late sender - ## bails here instead of dispatching onto a slot about to be reused. + ## bails here instead of dispatching onto a context about to be reused. if ctx.lifecycle.load() != CtxLifecycle.Active: deleteRequest(ffiRequest) return err("FFI context is not accepting requests (being recycled)") @@ -293,7 +293,7 @@ proc recycleContext[T]( ctx: ptr FFIContext[T], pending: ptr seq[Future[void]] ) {.async.} = ## Recycle handler, on the FFI thread (requestRecycle already closed the gate): - ## drain the in-flight handlers, free the lib object, release the slot for reuse, + ## drain the in-flight handlers, free the lib object, release the context for reuse, ## and fire the callback with the outcome. Never blocks the caller. ## ## `pending` is the run loop's seq of handler Futures, by ptr (async procs can't @@ -306,7 +306,7 @@ proc recycleContext[T]( naturallyDrained = await allFutures(pending[]).withTimeout(RecycleTimeout) ## 2. If any are wedged, cancel them and give the cancellations a bounded moment - ## to unwind, so the slot can be reclaimed rather than leaked. + ## to unwind, so the context can be reclaimed rather than leaked. var safeToRecycle = naturallyDrained if not naturallyDrained: for fut in pending[]: @@ -319,7 +319,7 @@ proc recycleContext[T]( ctx.recycleCallback = nil if safeToRecycle: - ## Nothing can touch the context now. Free the lib here, then release the slot + ## Nothing can touch the context now. Free the lib here, then release the context ## BEFORE the callback (the atomic store publishes these writes to whoever ## reclaims it) so a caller reacquiring on the callback finds it already free. freeLib(ctx) @@ -361,7 +361,7 @@ proc ffiThreadBody[T](ctx: ptr FFIContext[T]) {.thread.} = while ctx.running.load(): ## Recycle requested: claim it (RecyclePending -> Recycling, one-shot) and run ## the recycle on this owning thread, then keep looping so the worker stays - ## alive for the slot's next reuse. + ## alive for the context's next reuse. var expected = CtxLifecycle.RecyclePending if ctx.lifecycle.compareExchange(expected, CtxLifecycle.Recycling): await recycleContext(ctx, addr pending) @@ -425,9 +425,9 @@ proc cleanUpResources[T](ctx: ptr FFIContext[T]): Result[void, string] = return ok() proc initContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] = - ## Initialises all resources inside an already-allocated FFIContext slot. + ## Initialises all resources inside an already-allocated FFIContext. ## On failure every partially-initialised resource is closed; the caller - ## is responsible for releasing the slot (freeShared or pool.releaseSlot). + ## is responsible for releasing the context (freeShared or ctx.unclaim()). ctx.lock.initLock() var success = false @@ -491,7 +491,7 @@ proc signalStop*[T](ctx: ptr FFIContext[T]): Result[void, string] = ## If the FFI thread's event loop is blocked by a synchronous handler ## (e.g. blocking I/O), it cannot process reqSignal in time to exit. ## stopAndJoinThreads waits on threadExitSignal up to this bound; on timeout it -## returns err and skips joinThread/cleanup (leaking the thread + ctx slot) +## returns err and skips joinThread/cleanup (leaking the thread + ctx) ## rather than hanging the caller forever. const ThreadExitTimeout* = 1500.milliseconds @@ -537,22 +537,22 @@ proc requestRecycle*[T]( return ok() proc markReacquired*[T](ctx: ptr FFIContext[T]) = - ## Re-arms a recycled context when its slot is reacquired by createFFIContext: + ## Re-arms a recycled context when its context is reacquired by createFFIContext: ## moves Recycling -> Active (re-opening the gate). The FFI thread's `pending` ## seq was already drained and myLib freed by the recycle handler. ctx.lifecycle.store(CtxLifecycle.Active) proc tryClaim*[T](ctx: ptr FFIContext[T]): bool = - ## Atomically claim this slot (false -> true). Returns true if we won it, false - ## if it was already claimed. Used by createFFIContext to hand out a free slot. + ## Atomically claim this context (false -> true). Returns true if we won it, false + ## if it was already claimed. Used by createFFIContext to hand out a free context. var expected = false ctx.inUse.compareExchange(expected, true) proc unclaim*[T](ctx: ptr FFIContext[T]) = - ## Mark the slot free for reuse. Called by the recycle handler on the FFI thread + ## Mark the context free for reuse. Called by the recycle handler on the FFI thread ## once teardown is done, and on creation failure / full teardown. ctx.inUse.store(false) proc isClaimed*[T](ctx: ptr FFIContext[T]): bool = - ## Whether the slot is currently claimed by a consumer. + ## Whether the context is currently claimed by a consumer. ctx.inUse.load() diff --git a/ffi/ffi_context_pool.nim b/ffi/ffi_context_pool.nim index 1656317..acfb72c 100644 --- a/ffi/ffi_context_pool.nim +++ b/ffi/ffi_context_pool.nim @@ -4,34 +4,34 @@ import ./ffi_context, ./ffi_types const MaxFFIContexts* = 32 ## Maximum number of concurrently live FFI contexts when using FFIContextPool. - ## Fds and threads are only consumed for slots that are actually acquired, + ## Fds and threads are only consumed for contexts that are actually acquired, ## so this value only affects the upfront memory of the pool array. type FFIContextPool*[T] = object ## Fixed-size pool of FFI contexts. Avoids dynamic heap allocation per context ## and bounds the total number of file descriptors consumed by ThreadSignalPtrs ## to at most MaxFFIContexts * 2. - slots: array[MaxFFIContexts, FFIContext[T]] + contexts: array[MaxFFIContexts, FFIContext[T]] initialized: array[MaxFFIContexts, Atomic[bool]] - ## Whether a slot's worker (threads, chronos dispatcher and ThreadSignalPtrs) + ## Whether a context's worker (threads, chronos dispatcher and ThreadSignalPtrs) ## has been built. Set on first acquisition and kept set across park/reuse, - ## so a reacquired slot reuses the same fds instead of allocating a fresh set + ## so a reacquired context reuses the same fds instead of allocating a fresh set ## every create/destroy cycle. Cleared only by full teardown. proc createFFIContext*[T]( pool: var FFIContextPool[T] ): Result[ptr FFIContext[T], string] = - ## Acquires a slot from the fixed pool. The slot's worker is built once on - ## first use and REUSED on every later acquisition of the same slot (a slot is + ## Acquires a context from the fixed pool. The context's worker is built once on + ## first use and REUSED on every later acquisition of the same context (a context is ## made reacquirable by releaseFFIContext, which parks it without tearing the ## worker down). This is what keeps fd usage bounded: repeated create/destroy ## cycles no longer leak a fresh set of ThreadSignalPtr/dispatcher fds. for i in 0 ..< MaxFFIContexts: - let ctx = pool.slots[i].addr + let ctx = pool.contexts[i].addr if not ctx.tryClaim(): continue if pool.initialized[i].load(): - ## Reused slot: a prior destroy drained and released it, worker still alive. + ## Reused context: a prior destroy drained and released it, worker still alive. ## Re-arm the gate and hand it back. ctx.markReacquired() return ok(ctx) @@ -50,35 +50,35 @@ proc releaseFFIContext*[T]( ## for the generated destructor; destroyFFIContext is for failure/non-pool use. ## ## NON-BLOCKING: the FFI thread drains the handlers, frees the lib and releases - ## the slot, then fires `callback` (RET_OK drained, RET_ERR stuck). The slot - ## returns to the pool from that thread, so a reused slot never carries a straggler. + ## the context, then fires `callback` (RET_OK drained, RET_ERR stuck). The context + ## returns to the pool from that thread, so a reused context never carries a straggler. return ctx.requestRecycle(callback, userData) proc destroyFFIContext*[T]( pool: var FFIContextPool[T], ctx: ptr FFIContext[T] ): Result[void, string] = - ## Full teardown: stops/joins the worker threads and returns the slot to the + ## Full teardown: stops/joins the worker threads and returns the context to the ## pool, marking it uninitialised so a later createFFIContext rebuilds it. Used ## on creation failure and by non-pooling callers; steady-state cleanup should ## use releaseFFIContext to keep fd usage bounded. If the FFI thread is blocked - ## and does not exit in time, the slot is leaked rather than reclaimed — + ## and does not exit in time, the context is leaked rather than reclaimed — ## closing its resources while the thread is still live would be unsafe. ctx.stopAndJoinThreads().isOkOr: return err("destroyFFIContext(pool): " & $error) for i in 0 ..< MaxFFIContexts: - if pool.slots[i].addr == ctx: + if pool.contexts[i].addr == ctx: pool.initialized[i].store(false) break ctx.unclaim() return ok() proc isValidCtx*[T](pool: var FFIContextPool[T], ctx: pointer): bool = - ## Returns true only if ctx points to one of the pool's slots that is + ## Returns true only if ctx points to one of the pool's contexts that is ## currently in use. Rejects nil, offset-invalid, and dangling pointers ## at the API boundary, preventing use-after-free dereferences. if ctx.isNil(): return false for i in 0 ..< MaxFFIContexts: - if cast[pointer](pool.slots[i].addr) == ctx: + if cast[pointer](pool.contexts[i].addr) == ctx: return cast[ptr FFIContext[T]](ctx).isClaimed() return false diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index d770407..b417e2b 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -1517,7 +1517,7 @@ macro ffiDtor*(prc: untyped): untyped = ## The generated C-exported proc has the signature: ## cint mylibobj_destroy(void* ctx, FfiCallback callback, void* userData) ## - ## Recycle the slot for reuse to keep fd usage bounded. + ## Recycle the context for reuse to keep fd usage bounded. ## NON-BLOCKING: returns RET_OK once accepted; ## the real outcome arrives via `callback`. diff --git a/tests/test_ffi_context.nim b/tests/test_ffi_context.nim index b9876f3..bf6989c 100644 --- a/tests/test_ffi_context.nim +++ b/tests/test_ffi_context.nim @@ -129,18 +129,18 @@ suite "FFIContextPool": return check pool.destroyFFIContext(ctx).isOk() - test "slot is reused after destroy": + test "context is reused after destroy": var pool: FFIContextPool[TestLib] let ctx1 = pool.createFFIContext().valueOr: assert false, "createFFIContext(pool) failed: " & $error return check pool.destroyFFIContext(ctx1).isOk() - # After destroying, the same slot must be available again + # After destroying, the same context must be available again let ctx2 = pool.createFFIContext().valueOr: - assert false, "createFFIContext(pool) failed after slot release: " & $error + assert false, "createFFIContext(pool) failed after context release: " & $error return check pool.destroyFFIContext(ctx2).isOk() - check ctx1 == ctx2 # same array slot reused + check ctx1 == ctx2 # same context reused test "pool exhaustion returns error": var pool: FFIContextPool[TestLib] @@ -149,7 +149,7 @@ suite "FFIContextPool": ctxs[i] = pool.createFFIContext().valueOr: for j in 0 ..< i: discard pool.destroyFFIContext(ctxs[j]) - assert false, "createFFIContext(pool) failed at slot " & $i & ": " & $error + assert false, "createFFIContext(pool) failed at context " & $i & ": " & $error return # Pool is now full — next create must fail check pool.createFFIContext().isErr() @@ -471,7 +471,7 @@ proc createSimpleLib(initialValue: int): ptr FFIContext[SimpleLib] = return cast[ptr FFIContext[SimpleLib]](cast[uint](parseBiggestUInt(callbackMsg(d)))) suite "ffiDtor macro (async destroy + reuse)": - test "destroy fires RET_OK after teardown, frees myLib, and frees the slot": + test "destroy fires RET_OK after teardown, frees myLib, and frees the context": let ctx = createSimpleLib(5) check not ctx[].myLib.isNil check ctx[].myLib[].value == 5 @@ -489,9 +489,9 @@ suite "ffiDtor macro (async destroy + reuse)": check gDestroyedValue == 5 # the user cleanup body saw the live lib check ctx[].myLib.isNil() # freed on the FFI thread - # The slot was freed from the FFI thread, so a fresh create reclaims it. + # The context was freed from the FFI thread, so a fresh create reclaims it. let ctx2 = createSimpleLib(9) - check ctx2 == ctx # same slot, reused worker + fds + check ctx2 == ctx # same context, reused worker + fds check ctx2[].myLib[].value == 9 check SimpleLibFFIPool.destroyFFIContext(ctx2).isOk() @@ -531,7 +531,7 @@ suite "ffiDtor macro (async destroy + reuse)": waitCallback(dD) check dD.retCode == RET_OK - # Gate stays closed until the slot is reacquired: a late request must not + # Gate stays closed until the context is reacquired: a late request must not # dispatch onto a context about to be (or already) reused. var d: CallbackData initCallbackData(d) @@ -568,8 +568,8 @@ suite "ffiDtor macro (async destroy + reuse)": waitCallback(dD) check dD.retCode == RET_ERR # drain timed out -> ctx reported stuck - # The stuck slot is leaked (not reused); the handler still finishes on its - # own. Wait for it, then fully tear the leaked slot down. + # The stuck context is leaked (not reused); the handler still finishes on its + # own. Wait for it, then fully tear the leaked context down. waitCallback(slow) check SimpleLibFFIPool.destroyFFIContext(ctx).isOk() @@ -798,7 +798,7 @@ proc countOpenFds(): int = proc releaseAndWait[T](ctx: ptr FFIContext[T]): cint = ## Test helper mirroring how a C consumer destroys a context: kick off the ## (non-blocking) teardown and block on the callback, returning its retCode. - ## RET_OK means the lib's in-flight tasks finished and the slot was parked. + ## RET_OK means the lib's in-flight tasks finished and the context was parked. var d: CallbackData initCallbackData(d) defer: @@ -809,14 +809,14 @@ proc releaseAndWait[T](ctx: ptr FFIContext[T]): cint = return d.retCode suite "releaseFFIContext (park & reuse)": - test "park returns the slot and reuses the same live worker": + test "park returns the context and reuses the same live worker": var pool: FFIContextPool[TestLib] let ctx1 = pool.createFFIContext().valueOr: check false return check ctx1.releaseAndWait() == RET_OK - # Reacquire: must be the same array slot, with its worker still running. + # Reacquire: must be the same context, with its worker still running. let ctx2 = pool.createFFIContext().valueOr: check false return @@ -856,7 +856,7 @@ suite "releaseFFIContext (park & reuse)": else: var pool: FFIContextPool[TestLib] - # Warm up: the first create builds the slot's worker (its fds are allocated + # Warm up: the first create builds the context's worker (its fds are allocated # once here); parking keeps them open for reuse. block: let ctx = pool.createFFIContext().valueOr: @@ -885,7 +885,7 @@ suite "releaseFFIContext (park & reuse)": # only tolerates unrelated runtime fd noise, not a per-cycle leak. check afterCycles <= baseline + 5 - # Tear the (still parked) slot's worker down so the test leaves no threads. + # Tear the (still parked) context's worker down so the test leaves no threads. let last = pool.createFFIContext().valueOr: check false return