diff --git a/examples/echo/cpp_bindings/echo.hpp b/examples/echo/cpp_bindings/echo.hpp index d864346..0c4c965 100644 --- a/examples/echo/cpp_bindings/echo.hpp +++ b/examples/echo/cpp_bindings/echo.hpp @@ -332,7 +332,8 @@ void* echo_create(const uint8_t* req_cbor, size_t req_cbor_len, FFICallback call int echo_shout(void* ctx, FFICallback callback, void* user_data, const uint8_t* req_cbor, size_t req_cbor_len); int echo_version(void* ctx, FFICallback callback, void* user_data, const uint8_t* req_cbor, size_t req_cbor_len); int echo_destroy(void* ctx); -void echo_set_event_callback(void* ctx, FFICallback callback, void* user_data); +uint64_t echo_add_event_listener(void* ctx, const char* event_name, FFICallback callback, void* user_data); +int echo_remove_event_listener(void* ctx, uint64_t listener_id); } // extern "C" // ============================================================ diff --git a/examples/timer/cpp_bindings/my_timer.hpp b/examples/timer/cpp_bindings/my_timer.hpp index 2c5d9a3..dfc0c69 100644 --- a/examples/timer/cpp_bindings/my_timer.hpp +++ b/examples/timer/cpp_bindings/my_timer.hpp @@ -631,7 +631,8 @@ int my_timer_version(void* ctx, FFICallback callback, void* user_data, const uin int my_timer_complex(void* ctx, FFICallback callback, void* user_data, const uint8_t* req_cbor, size_t req_cbor_len); int my_timer_schedule(void* ctx, FFICallback callback, void* user_data, const uint8_t* req_cbor, size_t req_cbor_len); int my_timer_destroy(void* ctx); -void my_timer_set_event_callback(void* ctx, FFICallback callback, void* user_data); +uint64_t my_timer_add_event_listener(void* ctx, const char* event_name, FFICallback callback, void* user_data); +int my_timer_remove_event_listener(void* ctx, uint64_t listener_id); } // extern "C" // ============================================================ @@ -746,8 +747,13 @@ public: }; void setEventHandlers(Events handlers) { + if (event_listener_id_ != 0) { + my_timer_remove_event_listener(ptr_, event_listener_id_); + event_listener_id_ = 0; + } events_ = std::make_unique(std::move(handlers)); - my_timer_set_event_callback(ptr_, &MyTimerCtx::eventTrampoline, events_.get()); + event_listener_id_ = my_timer_add_event_listener( + ptr_, "", &MyTimerCtx::eventTrampoline, events_.get()); } EchoResponse echo(const EchoRequest& req) const { @@ -806,6 +812,7 @@ private: void* ptr_; std::chrono::milliseconds timeout_; std::unique_ptr events_; + uint64_t event_listener_id_ = 0; explicit MyTimerCtx(void* p, std::chrono::milliseconds t) : ptr_(p), timeout_(t) {} static void eventTrampoline(int ret, const char* msg, std::size_t len, void* ud) { if (!ud) return; diff --git a/examples/timer/rust_bindings/src/api.rs b/examples/timer/rust_bindings/src/api.rs index deb1fe2..6794d37 100644 --- a/examples/timer/rust_bindings/src/api.rs +++ b/examples/timer/rust_bindings/src/api.rs @@ -154,6 +154,7 @@ pub struct MyTimerCtx { ptr: *mut c_void, timeout: Duration, events: *mut Events, + event_listener_id: u64, } // SAFETY: The `ptr` field points to an FFIContext owned by the Nim runtime. @@ -190,7 +191,7 @@ impl MyTimerCtx { })?; let addr_str: String = decode_cbor(&raw_bytes)?; let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?; - Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut() }) + Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut(), event_listener_id: 0 }) } pub async fn new_async(config: TimerConfig, timeout: Duration) -> Result { @@ -202,14 +203,19 @@ impl MyTimerCtx { }).await?; let addr_str: String = decode_cbor(&raw_bytes)?; let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?; - Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut() }) + Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut(), event_listener_id: 0 }) } - /// Attach typed event handlers. Replacing handlers calls the - /// dylib's set_event_callback with a fresh trampoline target. - /// The previously-installed Events box (if any) is dropped here, - /// so callbacks in flight on the FFI thread must already be done. + /// Attach typed event handlers. Each call removes any previous + /// listener via `_remove_event_listener` before adding the new + /// one, so the registry never holds a pointer into a freed box. pub fn set_event_handlers(&mut self, handlers: Events) { + if self.event_listener_id != 0 { + unsafe { + let _ = ffi::my_timer_remove_event_listener(self.ptr, self.event_listener_id); + } + self.event_listener_id = 0; + } if !self.events.is_null() { unsafe { drop(Box::from_raw(self.events)); } self.events = std::ptr::null_mut(); @@ -217,7 +223,9 @@ impl MyTimerCtx { let raw = Box::into_raw(Box::new(handlers)); self.events = raw; unsafe { - ffi::my_timer_set_event_callback(self.ptr, my_timer_event_trampoline, raw as *mut c_void); + self.event_listener_id = ffi::my_timer_add_event_listener( + self.ptr, b"\0".as_ptr() as *const c_char, + my_timer_event_trampoline, raw as *mut c_void); } } diff --git a/examples/timer/rust_bindings/src/ffi.rs b/examples/timer/rust_bindings/src/ffi.rs index 40613ee..905acf4 100644 --- a/examples/timer/rust_bindings/src/ffi.rs +++ b/examples/timer/rust_bindings/src/ffi.rs @@ -15,5 +15,6 @@ extern "C" { pub fn my_timer_complex(ctx: *mut c_void, callback: FFICallback, user_data: *mut c_void, req_cbor: *const u8, req_cbor_len: usize) -> c_int; pub fn my_timer_schedule(ctx: *mut c_void, callback: FFICallback, user_data: *mut c_void, req_cbor: *const u8, req_cbor_len: usize) -> c_int; pub fn my_timer_destroy(ctx: *mut c_void) -> c_int; - pub fn my_timer_set_event_callback(ctx: *mut c_void, callback: FFICallback, user_data: *mut c_void); + pub fn my_timer_add_event_listener(ctx: *mut c_void, event_name: *const c_char, callback: FFICallback, user_data: *mut c_void) -> u64; + pub fn my_timer_remove_event_listener(ctx: *mut c_void, listener_id: u64) -> c_int; } diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index 9c5d3fd..30cef9f 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -161,10 +161,18 @@ proc emitEventDispatcher( lines.add(" };") lines.add("") lines.add(" void setEventHandlers(Events handlers) {") - lines.add(" events_ = std::make_unique(std::move(handlers));") + # Drop the previously-registered listener so the registry never holds + # a dangling pointer into a freed `Events` heap object. + lines.add(" if (event_listener_id_ != 0) {") lines.add( - " $1_set_event_callback(ptr_, &$2::eventTrampoline, events_.get());" % - [libName, ctxTypeName] + " $1_remove_event_listener(ptr_, event_listener_id_);" % [libName] + ) + lines.add(" event_listener_id_ = 0;") + lines.add(" }") + lines.add(" events_ = std::make_unique(std::move(handlers));") + lines.add(" event_listener_id_ = $1_add_event_listener(" % [libName]) + lines.add( + " ptr_, \"\", &$1::eventTrampoline, events_.get());" % [ctxTypeName] ) lines.add(" }") lines.add("") @@ -303,13 +311,15 @@ proc generateCppHeader*( ) of FFIKind.DTOR: lines.add("int $1(void* ctx);" % [p.procName]) - # The event-callback setter is always exported by the dylib (via - # declareLibrary). Declare it here so the typed event-handler wiring - # below can call into it. + # `declareLibrary` always exports the listener-registration ABI; + # declare it here so the typed event-handler wiring below can call in. lines.add( - "void $1_set_event_callback(void* ctx, FFICallback callback, void* user_data);" % + "uint64_t $1_add_event_listener(void* ctx, const char* event_name, FFICallback callback, void* user_data);" % [libName] ) + lines.add( + "int $1_remove_event_listener(void* ctx, uint64_t listener_id);" % [libName] + ) lines.add("} // extern \"C\"") lines.add("") @@ -501,6 +511,7 @@ proc generateCppHeader*( lines.add(" std::chrono::milliseconds timeout_;") if events.len > 0: lines.add(" std::unique_ptr events_;") + lines.add(" uint64_t event_listener_id_ = 0;") lines.add( " explicit $1(void* p, std::chrono::milliseconds t) : ptr_(p), timeout_(t) {}" % [ctxTypeName] diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 9ce0b5c..20c46c0 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -207,10 +207,14 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = params.add("ctx: *mut c_void") lines.add(" pub fn $1($2) -> c_int;" % [p.procName, params.join(", ")]) - # Event-callback setter — emitted on the Nim side by `declareLibrary`, - # always present in the dylib. + # Listener-registration ABI — emitted by `declareLibrary`, always + # present in the dylib. lines.add( - " pub fn $1_set_event_callback(ctx: *mut c_void, callback: FFICallback, user_data: *mut c_void);" % + " pub fn $1_add_event_listener(ctx: *mut c_void, event_name: *const c_char, callback: FFICallback, user_data: *mut c_void) -> u64;" % + [linkLibName] + ) + lines.add( + " pub fn $1_remove_event_listener(ctx: *mut c_void, listener_id: u64) -> c_int;" % [linkLibName] ) @@ -523,6 +527,7 @@ proc generateApiRs*( lines.add(" timeout: Duration,") if events.len > 0: lines.add(" events: *mut Events,") + lines.add(" event_listener_id: u64,") lines.add("}") lines.add("") # SAFETY block applies to both impls below (PR #23 Rust review, item 7). @@ -622,7 +627,7 @@ proc generateApiRs*( " let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?;" ) if events.len > 0: - lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut() })") + lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut(), event_listener_id: 0 })") else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") @@ -648,7 +653,7 @@ proc generateApiRs*( " let addr: usize = addr_str.parse().map_err(|e: std::num::ParseIntError| e.to_string())?;" ) if events.len > 0: - lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut() })") + lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, events: std::ptr::null_mut(), event_listener_id: 0 })") else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") @@ -656,11 +661,25 @@ proc generateApiRs*( # ── Typed event registration ─────────────────────────────────────────── if events.len > 0: - lines.add(" /// Attach typed event handlers. Replacing handlers calls the") - lines.add(" /// dylib's set_event_callback with a fresh trampoline target.") - lines.add(" /// The previously-installed Events box (if any) is dropped here,") - lines.add(" /// so callbacks in flight on the FFI thread must already be done.") + lines.add( + " /// Attach typed event handlers. Each call removes any previous" + ) + lines.add( + " /// listener via `_remove_event_listener` before adding the new" + ) + lines.add( + " /// one, so the registry never holds a pointer into a freed box." + ) lines.add(" pub fn set_event_handlers(&mut self, handlers: Events) {") + lines.add(" if self.event_listener_id != 0 {") + lines.add(" unsafe {") + lines.add( + " let _ = ffi::$1_remove_event_listener(self.ptr, self.event_listener_id);" % + [libName] + ) + lines.add(" }") + lines.add(" self.event_listener_id = 0;") + lines.add(" }") lines.add(" if !self.events.is_null() {") lines.add(" unsafe { drop(Box::from_raw(self.events)); }") lines.add(" self.events = std::ptr::null_mut();") @@ -669,9 +688,13 @@ proc generateApiRs*( lines.add(" self.events = raw;") lines.add(" unsafe {") lines.add( - " ffi::$1_set_event_callback(self.ptr, $1_event_trampoline, raw as *mut c_void);" % + " self.event_listener_id = ffi::$1_add_event_listener(" % [libName] ) + lines.add(" self.ptr, b\"\\0\".as_ptr() as *const c_char,") + lines.add( + " $1_event_trampoline, raw as *mut c_void);" % [libName] + ) lines.add(" }") lines.add(" }") lines.add("") diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index a572912..0fcd26c 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -108,51 +108,85 @@ macro declareLibraryBase*(libraryName: static[string]): untyped = return res macro declareLibrary*(libraryName: static[string], libType: untyped): untyped = - ## Declares a library with the given name and automatically generates - ## `{libraryName}_set_event_callback`, a C-exported function that stores the - ## caller's event callback on the FFIContext. + ## Declares a library with the given name and emits the C-exported event + ## ABI on its `FFIContext`: ## - ## `libType` is the Nim type of the main library object (e.g. `Waku`). It is used - ## to type the `ctx: ptr FFIContext[libType]` parameter of the generated - ## `{libraryName}_set_event_callback` proc. + ## - `{libraryName}_add_event_listener(ctx, event_name, cb, ud) -> uint64` + ## — registers `cb` for `event_name` and returns its stable id. An + ## empty `event_name` subscribes `cb` to *every* event (catch-all). + ## - `{libraryName}_remove_event_listener(ctx, id) -> cint` — returns 0 on + ## success, non-zero if no listener with that id exists. + ## + ## `libType` is the Nim type of the main library object, used to type + ## the `ctx: ptr FFIContext[libType]` parameter. See + ## `examples/timer/timer.nim` for a working call site. var stmts = newStmtList() # Emit the base bootstrap (pragmas, linker flags, NimMain, initializeLibrary) stmts.add(newCall(ident("declareLibraryBase"), newStrLitNode(libraryName))) - let funcName = libraryName & "_set_event_callback" - let funcIdent = ident(funcName) - let errorMsg = "error: invalid context in " & funcName - let ctxType = nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), libType)) - - let procBody = quote: - if isNil(ctx): - echo `errorMsg` - return - removeAllEventListeners(ctx[].eventRegistry) - if not callback.isNil(): - discard addEventListener( - ctx[].eventRegistry, WildcardEventName, callback, userData - ) - - let procNode = newProc( - name = funcIdent, - params = @[ - newEmptyNode(), - newIdentDefs(ident("ctx"), ctxType), - newIdentDefs(ident("callback"), ident("FFICallBack")), - newIdentDefs(ident("userData"), ident("pointer")), - ], - body = procBody, - pragmas = newTree( - nnkPragma, - ident("dynlib"), - ident("exportc"), - ident("cdecl"), - newTree(nnkExprColonExpr, ident("raises"), newTree(nnkBracket)), - ), + let cdeclExportPragma = newTree( + nnkPragma, + ident("dynlib"), + ident("exportc"), + ident("cdecl"), + newTree(nnkExprColonExpr, ident("raises"), newTree(nnkBracket)), + ) + + # {libraryName}_add_event_listener + let addName = libraryName & "_add_event_listener" + let addErr = "error: invalid context in " & addName + let addBody = quote: + var ret: uint64 = 0 + if isNil(ctx): + echo `addErr` + return ret + let evtName = if eventName.isNil(): "" else: $eventName + ret = addEventListener(ctx[].eventRegistry, evtName, callback, userData) + return ret + + stmts.add( + newProc( + name = ident(addName), + params = @[ + ident("uint64"), + newIdentDefs(ident("ctx"), ctxType), + newIdentDefs(ident("eventName"), ident("cstring")), + newIdentDefs(ident("callback"), ident("FFICallBack")), + newIdentDefs(ident("userData"), ident("pointer")), + ], + body = addBody, + pragmas = cdeclExportPragma, + ) + ) + + # --- {libraryName}_remove_event_listener -------------------------------- + # Param is `listenerId`, not `id` — `id` collides with chronos's + # `futures.id` template under quote injection rules and the captured + # symbol wins over the injected one. + let removeName = libraryName & "_remove_event_listener" + let removeErr = "error: invalid context in " & removeName + let removeBody = quote: + var ret: cint = 1 + if isNil(ctx): + echo `removeErr` + return ret + if removeEventListener(ctx[].eventRegistry, listenerId): + ret = 0 + return ret + + stmts.add( + newProc( + name = ident(removeName), + params = @[ + ident("cint"), + newIdentDefs(ident("ctx"), ctxType), + newIdentDefs(ident("listenerId"), ident("uint64")), + ], + body = removeBody, + pragmas = cdeclExportPragma, + ) ) - stmts.add(procNode) return stmts