feat(ffi): async teardown hook for {.ffiDtor.}#115
Open
gmelodie wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lets a library run async cleanup on the FFI worker thread during context teardown, and makes
destroyblock until it finishes. This is what unblocks anasynclibp2pDestroythatawaitsswitch.stop()so the switch is shut down gracefully — on the loop its chronos futures actually live on — before the FFI/event threads are joined and the context is freed.Before this, a
{.ffiDtor.}body ran synchronously on the host thread, with no seam to run async work on the worker at shutdown.Mechanism
A non-empty
{.ffiDtor.}body is lifted into an async impl registered in a per-library runtime hook slot (ffiTeardownHook[T](), one{.global.}per library type). After the FFI thread drains in-flight requests and before it exits, itawaits the hook on its own event loop. Exceptions are logged, never propagated — the thread must still firethreadExitSignaland exit cleanly.Why a runtime slot and not an overloaded
ffiTeardownresolved viamixin(the obvious design): the constructor force-instantiatesffiThreadBody[T]at itscreateFFIContextcall, which happens before the dtor — declared later in the source, as every consumer writes it (ctor then dtor) — is visible. An overload would bind the no-op default at that instantiation, so the teardown would compile clean,destroywould returnRET_OK, and the body would silently never run. Confirmed empirically: natural ctor-before-dtor order skips the teardown; dtor-first runs it. The runtime slot is assigned at module init and read at destroy, with zero compile-time ordering coupling.Changes
ffi/ffi_context.nim—FFITeardownProc[T]+ffiTeardownHook[T]()runtime slot.ThreadExitTimeoutis now configurable via-d:ffiThreadExitTimeoutMs=<ms>(default unchanged at 1500): a gracefulswitch.stop()over many live connections can outlast the old fixed bound, and being cut short leaks the context instead of waiting.ffi/ffi_thread.nim— await the registered hook at shutdown, guarded on hook-set andmyLib-populated.ffi/internal/ffi_macro.nim—{.ffiDtor.}now accepts a sync (void) or async (Future[void]) signature, validated up front. Non-empty bodies are lifted into the hook impl; the C wrapper is reduced to nil-check +destroyFFIContext. An empty/discardbody registers no hook, so existing no-op dtors (echo, timer, waku) are unchanged.Testing
tests/unit/test_ffi_teardown.nim(new) — assertsdestroyblocks until the async teardown body completes, and that it runs on the FFI thread (not the caller). Green under both--mm:orcand--mm:refc.discarddtors still compile and run;nimble check_bindings_cshows generated headers are unchanged; timer/echo examples still build; the-d:ffiThreadExitTimeoutMsknob compiles and takes effect.Rollout note
Consumer PR: vacp2p/nim-libp2p#2773 (vacp2p/nim-libp2p#2773) re-pins nim-ffi to this commit and lands the async
libp2pDestroy+ sharedshutdownSwitch+stoppedguard.That side should build with a larger
-d:ffiThreadExitTimeoutMs— the default 1500 ms is likely too short for a graceful multi-connectionswitch.stop(), and a teardown that outlasts the bound leaks the context (returnsRET_ERR) rather than hanging the caller.