chore(c): ctx_destroy returns int (#135)

CHANGELOG.md

@@ -5,6 +5,17 @@ All notable changes to this project are documented in this file.
 ## [Unreleased]
 
 ### Changed
+- The generated C `<lib>_ctx_destroy()` now returns `int` instead of `void`,
+  propagating the exported `<lib>_destroy()` status code (`NIMFFI_RET_OK` on
+  success, `RET_ERR` on a null/invalid context or a failed context teardown)
+  instead of discarding it, so a host can observe a failed teardown. Existing
+  callers that invoke it as a statement are unaffected
+  ([#133](https://github.com/logos-messaging/nim-ffi/issues/133)).
+- A failed `<lib>_ctx_destroy()` no longer frees the event-listener boxes. A
+  non-`NIMFFI_RET_OK` teardown leaves the worker threads live, and they still
+  hold each box as callback `user_data`; the boxes are now leaked rather than
+  freed out from under a running event thread. The context struct and the
+  listener array are still freed unconditionally.
 - User event callbacks now run on a dedicated event thread fed by a
   bounded SPSC queue (default capacity 1024), so a slow listener can no
   longer block the FFI thread or concurrent `add_event_listener` /

examples/echo/c_abi_bindings/echo.h

@@ -118,10 +118,12 @@ static inline int echo_ctx_create(const EchoConfig* config, EchoCreateFn on_crea
     return 0;
 }
 
-static inline void echo_ctx_destroy(EchoCtx* ctx) {
-    if (!ctx) return;
-    if (ctx->ptr) { echo_destroy(ctx->ptr); ctx->ptr = NULL; }
+static inline int echo_ctx_destroy(EchoCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = echo_destroy(ctx->ptr); ctx->ptr = NULL; }
     free(ctx);
+    return rc;
 }
 
 static inline int echo_ctx_shout(const EchoCtx* ctx, const ShoutRequest* req, EchoShoutReplyFn on_reply, void* user_data) {

examples/echo/c_bindings/echo.h

@@ -289,10 +289,12 @@ static inline int echo_ctx_create(const EchoConfig* config, EchoCreateFn on_crea
     return 0;
 }
 
-static inline void echo_ctx_destroy(EchoCtx* ctx) {
-    if (!ctx) return;
-    if (ctx->ptr) { echo_destroy(ctx->ptr); ctx->ptr = NULL; }
+static inline int echo_ctx_destroy(EchoCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = echo_destroy(ctx->ptr); ctx->ptr = NULL; }
     free(ctx);
+    return rc;
 }
 
 typedef void (*EchoShoutReplyFn)(int err_code, const ShoutResponse* reply, const char* err_msg, void* user_data);

examples/timer/c_bindings/my_timer.h

@@ -896,12 +896,16 @@ static inline int my_timer_ctx_create(const TimerConfig* config, MyTimerCreateFn
     return 0;
 }
 
-static inline void my_timer_ctx_destroy(MyTimerCtx* ctx) {
-    if (!ctx) return;
-    if (ctx->ptr) { my_timer_destroy(ctx->ptr); ctx->ptr = NULL; }
-    for (size_t i = 0; i < ctx->listeners_len; i++) free(ctx->listeners[i].box);
+static inline int my_timer_ctx_destroy(MyTimerCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = my_timer_destroy(ctx->ptr); ctx->ptr = NULL; }
+    if (rc == NIMFFI_RET_OK) {
+        for (size_t i = 0; i < ctx->listeners_len; i++) free(ctx->listeners[i].box);
+    }
     free(ctx->listeners);
     free(ctx);
+    return rc;
 }
 
 static inline uint64_t my_timer_ctx_add_on_echo_fired_listener(MyTimerCtx* ctx, MyTimerOnEchoFiredFn fn, void* user_data) {

ffi/codegen/c.nim

@@ -528,16 +528,25 @@ proc emitDestructor(
     ctxType, libName, dtorProcName: string,
     events: seq[FFIEventMeta],
 ) =
-  lines.add("static inline void " & libName & "_ctx_destroy(" & ctxType & "* ctx) {")
-  lines.add("    if (!ctx) return;")
+  lines.add("static inline int " & libName & "_ctx_destroy(" & ctxType & "* ctx) {")
+  lines.add("    if (!ctx) return NIMFFI_RET_OK;")
+  lines.add("    int rc = NIMFFI_RET_OK;")
   if dtorProcName.len > 0:
-    lines.add("    if (ctx->ptr) { " & dtorProcName & "(ctx->ptr); ctx->ptr = NULL; }")
-  if events.len > 0:
     lines.add(
-      "    for (size_t i = 0; i < ctx->listeners_len; i++) free(ctx->listeners[i].box);"
+      "    if (ctx->ptr) { rc = " & dtorProcName & "(ctx->ptr); ctx->ptr = NULL; }"
     )
+  if events.len > 0:
+    # A failed teardown leaves the worker threads live (ffi_context.nim:
+    # stopAndJoinThreads), and they still hold each box as callback user_data.
+    # Leaking a box beats handing a running event thread a dangling pointer.
+    lines.add("    if (rc == NIMFFI_RET_OK) {")
+    lines.add(
+      "        for (size_t i = 0; i < ctx->listeners_len; i++) free(ctx->listeners[i].box);"
+    )
+    lines.add("    }")
     lines.add("    free(ctx->listeners);")
   lines.add("    free(ctx);")
+  lines.add("    return rc;")
   lines.add("}")
   lines.add("")
 
@@ -1194,15 +1203,6 @@ proc emitAbiCtxAndCtor(
     lines.add("}")
     lines.add("")
 
-proc emitAbiDestructor(lines: var seq[string], ctxType, libName, dtorProcName: string) =
-  lines.add("static inline void " & libName & "_ctx_destroy(" & ctxType & "* ctx) {")
-  lines.add("    if (!ctx) return;")
-  if dtorProcName.len > 0:
-    lines.add("    if (ctx->ptr) { " & dtorProcName & "(ctx->ptr); ctx->ptr = NULL; }")
-  lines.add("    free(ctx);")
-  lines.add("}")
-  lines.add("")
-
 proc emitAbiMethod(
     lines: var seq[string],
     reg: var AbiReg,
@@ -1400,7 +1400,8 @@ proc generateCAbiLibHeader*(
 
   lines.add("/* High-level context wrapper */")
   emitAbiCtxAndCtor(lines, reg, libName, libType, ctxType, classified.ctors)
-  emitAbiDestructor(lines, ctxType, libName, classified.dtorProcName)
+  # abi = c has no events, so the destructor is the CBOR one minus the listener sweep.
+  emitDestructor(lines, ctxType, libName, classified.dtorProcName, @[])
   for m in classified.methods:
     if m.scalarFastPath:
       emitAbiScalarMethod(lines, reg, ctxType, libName, libType, m)

tests/e2e/c/test_timer_e2e.c

@@ -257,7 +257,7 @@ int main(void) {
     test_schedule_ok(ctx);
     test_schedule_error(ctx);
     test_event(ctx);
-    my_timer_ctx_destroy(ctx);
+    assert(my_timer_ctx_destroy(ctx) == NIMFFI_RET_OK);
     printf("all C e2e checks passed\n");
     return 0;
 }

tests/e2e/c_abi/test_echo_c_abi.c

@@ -107,7 +107,7 @@ int main(void) {
     EchoCtx* ctx = make_ctx();
     test_shout(ctx);
     test_version(ctx);
-    echo_ctx_destroy(ctx);
+    assert(echo_ctx_destroy(ctx) == NIMFFI_RET_OK);
     printf("all abi=c echo e2e checks passed\n");
     return 0;
 }

tests/unit/test_c_abi_codegen.nim

@@ -133,6 +133,19 @@ suite "generateCAbiLibHeader":
     check "timer_ctx_version(" in header
     check "timer_ctx_destroy(" in header
 
+  test "the context destructor propagates the destructor's status code":
+    check(
+      """
+static inline int timer_ctx_destroy(TimerCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = timer_destroy(ctx->ptr); ctx->ptr = NULL; }
+    free(ctx);
+    return rc;
+}""" in
+        header
+    )
+
   test "scalar-fast-path methods take inline args, not a Req struct":
     check "TimerAddReq" notin header
     check "TimerNameReq" notin header

tests/unit/test_c_codegen.nim

@@ -124,6 +124,19 @@ suite "generateCLibHeader: ABI declarations and context API":
     check "timer_ctx_version(" in header
     check "timer_ctx_destroy(" in header
 
+  test "the context destructor propagates the destructor's status code":
+    check(
+      """
+static inline int timer_ctx_destroy(TimerCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = timer_destroy(ctx->ptr); ctx->ptr = NULL; }
+    free(ctx);
+    return rc;
+}""" in
+        header
+    )
+
   test "the async API is callback-driven, not blocking":
     # methods take a typed reply callback + user_data; no out-param, no char** err
     check "typedef void (*TimerVersionReplyFn)(int err_code, const NimFfiStr* reply, const char* err_msg, void* user_data);" in
@@ -192,6 +205,25 @@ suite "generateCLibHeader: events":
   test "the context tracks listeners only when events exist":
     check "TimerCtxListener* listeners;" in header
 
+  test "a failed teardown leaks the listener boxes instead of dangling them":
+    # A non-OK rc leaves the worker threads live, still holding each box as
+    # callback user_data, so the sweep must sit behind the rc guard.
+    check(
+      """
+static inline int timer_ctx_destroy(TimerCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    if (ctx->ptr) { rc = timer_destroy(ctx->ptr); ctx->ptr = NULL; }
+    if (rc == NIMFFI_RET_OK) {
+        for (size_t i = 0; i < ctx->listeners_len; i++) free(ctx->listeners[i].box);
+    }
+    free(ctx->listeners);
+    free(ctx);
+    return rc;
+}""" in
+        header
+    )
+
 suite "generateCLibHeader: no-event libraries stay lean":
   test "a library without events has no listener bookkeeping":
     let procs = @[
@@ -208,6 +240,29 @@ suite "generateCLibHeader: no-event libraries stay lean":
     check "listeners_len" notin header
     check "_add_event_listener" in header # raw ABI symbol is always declared
 
+  test "a library without a dtor still reports success from ctx_destroy":
+    let procs = @[
+      FFIProcMeta(
+        procName: "timer_create",
+        libName: "timer",
+        kind: FFIKind.CTOR,
+        libTypeName: "Timer",
+        extraParams: @[],
+        returnTypeName: "Timer",
+      )
+    ]
+    let header = generateCLibHeader(procs, @[], "timer")
+    check(
+      """
+static inline int timer_ctx_destroy(TimerCtx* ctx) {
+    if (!ctx) return NIMFFI_RET_OK;
+    int rc = NIMFFI_RET_OK;
+    free(ctx);
+    return rc;
+}""" in
+        header
+    )
+
 suite "generateCLibHeader: scalar-fast-path procs are excluded":
   setup:
     let procs = @[