fix(pool): park context on destroy to keep fd reuse active

The reuse branch in createFFIContext only fires for a parked slot, and a
slot is only parked by releaseFFIContext. The generated destructor was
still calling destroyFFIContext (full teardown, marks the slot
uninitialised), so the reuse path never triggered and the fd-leak fix was
inert in the generated API.

Switch ffiDtor to releaseFFIContext so the worker and its fds survive the
destroy and get reused on the next create. This is safe because the
framework handles one request at a time: by the time the destructor runs
the worker is idle, not mid-request, so parking cannot race a handler.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ivan FB 2026-06-04 14:53:36 +02:00
parent 1da81c42da
commit 1c99eaa189
No known key found for this signature in database
GPG Key ID: DF0C67A04C543270
2 changed files with 23 additions and 6 deletions

View File

@ -52,11 +52,22 @@ proc releaseFFIContext*[T](
pool: var FFIContextPool[T], ctx: ptr FFIContext[T]
): Result[void, string] =
## Parks a context for reuse: returns its slot to the pool WITHOUT stopping the
## worker threads, so the next createFFIContext reuses the same fds. The caller
## MUST have already quiesced its library object (e.g. cancelled the object's
## async tasks) on the FFI thread before calling this — the worker keeps
## running. The stale C event callback is dropped so a watchdog tick on the
## parked slot cannot invoke a callback whose user-data may already be freed.
## worker threads, so the next createFFIContext reuses the same fds. This is the
## steady-state cleanup path, called by the generated destructor (ffiDtor);
## destroyFFIContext is only for creation failure and non-pooling callers.
##
## Runs on the CALLER's thread, not the FFI worker thread, and does NOT itself
## wait for in-flight work to finish. It is safe to park here because the
## framework processes one request at a time (see sendRequestToFFIThread): by
## the time the destructor calls this, the worker has finished the previous
## request and is idle (looping on reqSignal), so there is no handler still
## touching the slot when it is reused.
##
## Clearing callbackState removes the stored C event callback. The
## worker/watchdog threads stay alive after parking, so an event could still
## fire on this slot; with no callback set that event does nothing, instead of
## calling back into a consumer that has already released the context (whose
## user-data pointer may now be freed).
ctx.callbackState = default(FFICallbackState)
ctx.myLib = nil
pool.releaseSlot(ctx)

View File

@ -1565,9 +1565,15 @@ macro ffiDtor*(prc: untyped): untyped =
ffiBody.add(bodyNode)
let poolIdent = ident($libTypeName & "FFIPool")
# Park the slot (releaseFFIContext) instead of tearing it down
# (destroyFFIContext) so the next createFFIContext reuses the same worker and
# fds — this is what keeps fd usage bounded across create/destroy cycles.
# Safe because the framework processes one request at a time
# (see sendRequestToFFIThread): by the time this destructor runs the worker is
# idle, not mid-request, so parking cannot race an in-flight handler.
ffiBody.add quote do:
let `destroyResIdent` =
`poolIdent`.destroyFFIContext(cast[ptr FFIContext[`libTypeName`]](ctx))
`poolIdent`.releaseFFIContext(cast[ptr FFIContext[`libTypeName`]](ctx))
if `destroyResIdent`.isErr():
if not callback.isNil:
let errStr = "destroy failed: " & $`destroyResIdent`.error