emitDestructor emits the destructor call with its return value dropped, and the wrapper itself is static inline void, so a host has no way to observe a failed teardown.
|
lines.add(" if (ctx->ptr) { " & dtorProcName & "(ctx->ptr); ctx->ptr = NULL; }") |
static inline void libp2p_ctx_destroy(LibP2PCtx* ctx) {
if (!ctx) return;
if (ctx->ptr) { libp2p_destroy(ctx->ptr); ctx->ptr = NULL; } // int
return discarded
for (size_t i = 0; i < ctx->listeners_len; i++)
free(ctx->listeners[i].box);
free(ctx->listeners);
free(ctx);
}
The exported destructor is declared as int libp2p_destroy(void* ctx), and its status codes are meaningful: NIMFFI_RET_OK (0) indicates success, while any non-zero value indicates failure. However, the return value is currently discarded. As a result, the C-side context wrapper and all listener boxes are freed unconditionally. If teardown actually fails, the caller has already torn down its side while the Nim side may still hold resources, and the failure is never reported. The same code is emitted in both C backends:
Suggested fix: Make <lib>_ctx_destroy return an int, propagating the destructor's return code while still freeing the C-side state. This preserves unconditional teardown while making failures observable. This change is source-compatible: existing callers that invoke *_ctx_destroy as a statement continue to compile unchanged, while callers that care about teardown failures can begin checking the return value.
emitDestructoremits the destructor call with its return value dropped, and the wrapper itself is static inline void, so a host has no way to observe a failed teardown.nim-ffi/ffi/codegen/c.nim
Line 558 in 7ef58f4
The exported destructor is declared as
int libp2p_destroy(void* ctx), and its status codes are meaningful:NIMFFI_RET_OK(0) indicates success, while any non-zero value indicates failure. However, the return value is currently discarded. As a result, the C-side context wrapper and all listener boxes are freed unconditionally. If teardown actually fails, the caller has already torn down its side while the Nim side may still hold resources, and the failure is never reported. The same code is emitted in both C backends:ffi/codegen/c.nim#L558ffi/codegen/c_abi.nim#L340This was found while reviewing feat: use nim-ffi cbindings logos-co/logos-libp2p-module#77. The host's
destroyContext()implementation simply callslibp2p_ctx_destroy(ctx), leaving no way for the caller to detect teardown failures.Suggested fix: Make
<lib>_ctx_destroyreturn anint, propagating the destructor's return code while still freeing the C-side state. This preserves unconditional teardown while making failures observable. This change is source-compatible: existing callers that invoke*_ctx_destroyas a statement continue to compile unchanged, while callers that care about teardown failures can begin checking the return value.