From 7ccf34591d89c7536afe8b68380f9b3d7dd44454 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz <8129788+gmelodie@users.noreply.github.com> Date: Fri, 29 May 2026 12:35:49 -0300 Subject: [PATCH 1/4] chore: avoid throwing exceptions in C++ bindings (#46) --- examples/echo/cpp_bindings/echo.hpp | 153 ++++++++--- examples/timer/cpp_bindings/main.cpp | 247 ++++++++++-------- examples/timer/cpp_bindings/my_timer.hpp | 179 +++++++++---- ffi/codegen/cpp.nim | 63 +++-- .../templates/cpp/cbor_helpers.hpp.tpl | 20 +- .../templates/cpp/header_prelude.hpp.tpl | 3 +- ffi/codegen/templates/cpp/result.hpp.tpl | 61 +++++ .../templates/cpp/sync_call_helper.hpp.tpl | 15 +- tests/e2e/cpp/test_timer_e2e.cpp | 125 +++++---- 9 files changed, 576 insertions(+), 290 deletions(-) create mode 100644 ffi/codegen/templates/cpp/result.hpp.tpl diff --git a/examples/echo/cpp_bindings/echo.hpp b/examples/echo/cpp_bindings/echo.hpp index 9281f09..c6f50d0 100644 --- a/examples/echo/cpp_bindings/echo.hpp +++ b/examples/echo/cpp_bindings/echo.hpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,10 +24,73 @@ #include #include #include +#include extern "C" { #include } +// ============================================================ +// Result — exception-free error channel +// ============================================================ +// The generated bindings never throw: every fallible entry point (create, +// instance methods, and their *Async futures) returns a Result. Callers +// branch on isOk()/isErr() (or the explicit bool conversion) and read +// value()/error(). This mirrors the Nim side's Result[T, string] and keeps +// us off C++23's std::expected. +#ifndef NIM_FFI_RESULT_HPP_INCLUDED +#define NIM_FFI_RESULT_HPP_INCLUDED + +template +class Result { + std::optional value_; + std::string error_; +public: + static Result ok(T value) { + Result r; + r.value_ = std::move(value); + return r; + } + static Result err(std::string message) { + Result r; + r.error_ = std::move(message); + return r; + } + bool isOk() const { return value_.has_value(); } + bool isErr() const { return !value_.has_value(); } + explicit operator bool() const { return isOk(); } + const T& value() const { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + T& value() { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + const T& operator*() const { assert(value_.has_value() && "Result::operator*() called on err Result — check isOk() first"); return *value_; } + const T* operator->() const { assert(value_.has_value() && "Result::operator->() called on err Result — check isOk() first"); return &*value_; } + T&& take() { assert(value_.has_value() && "Result::take() called on err Result — check isOk() first"); return std::move(*value_); } + const std::string& error() const { assert(!value_.has_value() && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +template <> +class Result { + bool ok_ = true; + std::string error_; +public: + static Result ok() { + Result r; + r.ok_ = true; + return r; + } + static Result err(std::string message) { + Result r; + r.ok_ = false; + r.error_ = std::move(message); + return r; + } + Result() = default; + bool isOk() const { return ok_; } + bool isErr() const { return !ok_; } + explicit operator bool() const { return isOk(); } + const std::string& error() const { assert(!ok_ && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +#endif // NIM_FFI_RESULT_HPP_INCLUDED + // ── encode_cbor overloads (primitives + containers) ───────────────────── // Per-struct encode_cbor / decode_cbor are emitted by cpp.nim next to each // generated struct; these helpers cover the leaf types they defer into. @@ -159,7 +222,7 @@ inline CborError decode_cbor(CborValue& it, std::optional& out) { // ── Public entry points ───────────────────────────────────────────────── template -inline std::vector encodeCborFFI(const T& value) { +inline Result> encodeCborFFI(const T& value) { // Start with a generous 4 KiB buffer; double on overflow until it fits. std::vector buf(4096); while (true) { @@ -169,34 +232,34 @@ inline std::vector encodeCborFFI(const T& value) { if (err == CborNoError) { const size_t used = cbor_encoder_get_buffer_size(&enc, buf.data()); buf.resize(used); - return buf; + return Result>::ok(std::move(buf)); } if (err == CborErrorOutOfMemory) { const size_t extra = cbor_encoder_get_extra_bytes_needed(&enc); buf.resize(buf.size() + (extra > 0 ? extra : buf.size())); continue; } - throw std::runtime_error(std::string("FFI CBOR encode failed: ") + - cbor_error_string(err)); + return Result>::err( + std::string("FFI CBOR encode failed: ") + cbor_error_string(err)); } } template -inline T decodeCborFFI(const std::vector& bytes) { +inline Result decodeCborFFI(const std::vector& bytes) { CborParser parser; CborValue it; CborError err = cbor_parser_init(bytes.data(), bytes.size(), 0, &parser, &it); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR parse init failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR parse init failed: ") + + cbor_error_string(err)); } T out{}; err = decode_cbor(it, out); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR decode failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR decode failed: ") + + cbor_error_string(err)); } - return out; + return Result::ok(std::move(out)); } #endif // NIM_FFI_CBOR_HELPERS_HPP_INCLUDED @@ -384,22 +447,25 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { s.cv.notify_one(); } -inline std::vector ffi_call_(std::function f, - std::chrono::milliseconds timeout) { +inline Result> ffi_call_( + std::function f, + std::chrono::milliseconds timeout) { + using Bytes = std::vector; auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); if (ret == 2) { delete cb_ref; - throw std::runtime_error("RET_MISSING_CALLBACK (internal error)"); + return Result::err("RET_MISSING_CALLBACK (internal error)"); } std::unique_lock lock(state->mtx); const bool fired = state->cv.wait_for(lock, timeout, [&]{ return state->done; }); if (!fired) - throw std::runtime_error("FFI call timed out after " + std::to_string(timeout.count()) + "ms"); + return Result::err("FFI call timed out after " + + std::to_string(timeout.count()) + "ms"); if (!state->ok) - throw std::runtime_error(state->err); - return state->bytes; + return Result::err(state->err); + return Result::ok(std::move(state->bytes)); } } // anonymous namespace @@ -412,23 +478,30 @@ inline std::vector ffi_call_(std::function create(const EchoConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { + static Result> create(const EchoConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { const auto ffi_req_ = EchoCreateCtorReq{config}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result>::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { (void)echo_create(ffi_req_bytes_.data(), ffi_req_bytes_.size(), cb, ud); return 0; }, timeout); - const auto addr_str = decodeCborFFI(ffi_raw_); - try { - const auto addr = std::stoull(addr_str); - return std::unique_ptr(new EchoCtx(reinterpret_cast(static_cast(addr)), timeout)); - } catch (const std::exception&) { - throw std::runtime_error("FFI create returned non-numeric address: " + addr_str); + if (ffi_raw_.isErr()) return Result>::err(ffi_raw_.error()); + auto ffi_addr_ = decodeCborFFI(ffi_raw_.value()); + if (ffi_addr_.isErr()) return Result>::err(ffi_addr_.error()); + const auto& addr_str = ffi_addr_.value(); + std::uint64_t addr = 0; + const char* addr_begin = addr_str.data(); + const char* addr_end = addr_begin + addr_str.size(); + const auto fc_ = std::from_chars(addr_begin, addr_end, addr); + if (fc_.ec != std::errc() || fc_.ptr != addr_end) { + return Result>::err("FFI create returned non-numeric address: " + addr_str); } + return Result>::ok(std::unique_ptr(new EchoCtx(reinterpret_cast(static_cast(addr)), timeout))); } - static std::future> createAsync(const EchoConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { + static std::future>> createAsync(const EchoConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { return std::async(std::launch::async, [config, timeout]() { return create(config, timeout); }); } @@ -453,29 +526,35 @@ public: EchoCtx(EchoCtx&&) = delete; EchoCtx& operator=(EchoCtx&&) = delete; - ShoutResponse shout(const ShoutRequest& req) const { + Result shout(const ShoutRequest& req) const { const auto ffi_req_ = EchoShoutReq{req}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return echo_shout(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future shoutAsync(const ShoutRequest& req) const { + std::future> shoutAsync(const ShoutRequest& req) const { return std::async(std::launch::async, [this, req]() { return this->shout(req); }); } - std::string version() const { + Result version() const { const auto ffi_req_ = EchoVersionReq{}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return echo_version(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future versionAsync() const { + std::future> versionAsync() const { return std::async(std::launch::async, [this]() { return this->version(); }); } diff --git a/examples/timer/cpp_bindings/main.cpp b/examples/timer/cpp_bindings/main.cpp index 5a9e7ea..3ab361d 100644 --- a/examples/timer/cpp_bindings/main.cpp +++ b/examples/timer/cpp_bindings/main.cpp @@ -5,121 +5,142 @@ #include #include +// The generated bindings never throw: every call returns a Result. We +// branch on isErr() and read value()/error() instead of using try/catch. int main() { - try { - auto ctx = MyTimerCtx::create(TimerConfig{"cpp-demo"}); - std::cout << "[1] Context created\n"; - - auto versionFuture = ctx->versionAsync(); - auto echo1Future = ctx->echoAsync(EchoRequest{"hello from C++", 200}); - auto echo2Future = ctx->echoAsync(EchoRequest{"second C++ request", 50}); - - auto version = versionFuture.get(); - std::cout << "[2] Version: " << version << "\n"; - - auto echo = echo1Future.get(); - std::cout << "[3] Echo 1: echoed=" << echo.echoed - << ", timerName=" << echo.timerName << "\n"; - - auto echo2 = echo2Future.get(); - std::cout << "[4] Echo 2: echoed=" << echo2.echoed - << ", timerName=" << echo2.timerName << "\n"; - - auto complexReq = ComplexRequest{ - std::vector{EchoRequest{"one", 10}, EchoRequest{"two", 20}}, - std::vector{"fast", "async"}, - std::optional("extra note"), - std::optional(3) - }; - - auto complexFuture = ctx->complexAsync(complexReq); - auto complex = complexFuture.get(); - std::cout << "[5] Complex: summary=" << complex.summary - << ", itemCount=" << complex.itemCount - << ", hasNote=" << complex.hasNote << "\n"; - - // Each parameter is its own generated C++ struct. The nim-ffi - // macro packs all three into one CBOR envelope on the wire — at - // the call site, this is just a typed method invocation. - auto job = JobSpec{ - /*name*/ "nightly-rollup", - /*payload*/ std::vector{"rollup", "v2"}, - /*priority*/ 10, - }; - auto retry = RetryPolicy{ - /*maxAttempts*/ 3, - /*backoffMs*/ 500, - /*retryOn*/ std::vector{"timeout", "5xx"}, - }; - auto schedule = ScheduleConfig{ - /*startAtMs*/ 1000, - /*intervalMs*/ 15000, - /*jitter*/ std::optional(250), - }; - - auto scheduleFuture = ctx->scheduleAsync(job, retry, schedule); - auto scheduleRes = scheduleFuture.get(); - std::cout << "[6] Schedule (3 complex params): jobId=" << scheduleRes.jobId - << ", willRunCount=" << scheduleRes.willRunCount - << ", firstRunAtMs=" << scheduleRes.firstRunAtMs - << ", effectiveBackoffMs=" << scheduleRes.effectiveBackoffMs - << "\n"; - - // Each `{.ffiEvent.}` declared on the Nim side gets a typed - // registration method — `addOnEchoFiredListener(handler)` here. - // A second `addEventListener` overload registers a catch-all - // wildcard listener that receives every event as raw envelope - // bytes plus the FFI return code. Both fire from the lib's - // dispatch thread, so synchronise via std::promise / atomics. - std::promise echoEvtPromise; - auto echoEvtFuture = echoEvtPromise.get_future(); - const auto typedHandle = ctx->addOnEchoFiredListener( - [&](const EchoEvent& evt) { echoEvtPromise.set_value(evt); }); - - std::atomic wildcardHits{0}; - // Wildcard listener receives every event with the wire `eventId` - // pre-extracted plus a span view over the raw CBOR envelope - // bytes (zero-copy; valid only for the duration of this call). - // Dispatch on `eventId` and use `decodeEventPayload` to lift - // the payload into a typed value without hand-parsing CBOR. - const auto wildcardHandle = ctx->addEventListener( - [&](int retCode, const std::string& eventId, - std::span envelope) { - wildcardHits.fetch_add(1); - std::cout << "[7] wildcard event: retCode=" << retCode - << ", eventId=" << eventId - << ", envelope bytes=" << envelope.size() << "\n"; - if (retCode != 0) return; - if (eventId == "on_echo_fired") { - EchoEvent decoded{}; - if (decodeEventPayload(envelope, decoded)) { - std::cout << " decoded EchoEvent: message=" - << decoded.message - << ", echoCount=" << decoded.echoCount << "\n"; - } - } - }); - - ctx->echo(EchoRequest{"event-demo", 1}); - const auto evt = echoEvtFuture.get(); - std::cout << "[7] typed event onEchoFired: message=" << evt.message - << ", echoCount=" << evt.echoCount - << ", wildcardHits=" << wildcardHits.load() << "\n"; - - // Drop the typed listener — only the wildcard fires for the - // follow-up echo. Sleep briefly to give the lib thread time to - // deliver before we tear the ctx down. - ctx->removeEventListener(typedHandle); - ctx->echo(EchoRequest{"event-demo-after-remove", 1}); - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - std::cout << "[7] after removeEventListener: wildcardHits=" - << wildcardHits.load() << "\n"; - ctx->removeEventListener(wildcardHandle); - - std::cout << "\nDone.\n"; - } catch (const std::exception& ex) { - std::cerr << "Error: " << ex.what() << "\n"; + auto ctxRes = MyTimerCtx::create(TimerConfig{"cpp-demo"}); + if (ctxRes.isErr()) { + std::cerr << "Error: " << ctxRes.error() << "\n"; return 1; } + auto ctx = std::move(ctxRes.value()); + std::cout << "[1] Context created\n"; + + auto versionFuture = ctx->versionAsync(); + auto echo1Future = ctx->echoAsync(EchoRequest{"hello from C++", 200}); + auto echo2Future = ctx->echoAsync(EchoRequest{"second C++ request", 50}); + + auto version = versionFuture.get(); + if (version.isErr()) { + std::cerr << "Error: " << version.error() << "\n"; + return 1; + } + std::cout << "[2] Version: " << version.value() << "\n"; + + auto echo = echo1Future.get(); + if (echo.isErr()) { + std::cerr << "Error: " << echo.error() << "\n"; + return 1; + } + std::cout << "[3] Echo 1: echoed=" << echo->echoed + << ", timerName=" << echo->timerName << "\n"; + + auto echo2 = echo2Future.get(); + if (echo2.isErr()) { + std::cerr << "Error: " << echo2.error() << "\n"; + return 1; + } + std::cout << "[4] Echo 2: echoed=" << echo2->echoed + << ", timerName=" << echo2->timerName << "\n"; + + auto complexReq = ComplexRequest{ + std::vector{EchoRequest{"one", 10}, EchoRequest{"two", 20}}, + std::vector{"fast", "async"}, + std::optional("extra note"), + std::optional(3) + }; + + auto complex = ctx->complexAsync(complexReq).get(); + if (complex.isErr()) { + std::cerr << "Error: " << complex.error() << "\n"; + return 1; + } + std::cout << "[5] Complex: summary=" << complex->summary + << ", itemCount=" << complex->itemCount + << ", hasNote=" << complex->hasNote << "\n"; + + // ── 6. Call with three complex parameters ───────────────────── + // Each parameter is its own generated C++ struct. The nim-ffi + // macro packs all three into one CBOR envelope on the wire — at + // the call site, this is just a typed method invocation. + auto job = JobSpec{ + /*name*/ "nightly-rollup", + /*payload*/ std::vector{"rollup", "v2"}, + /*priority*/ 10, + }; + auto retry = RetryPolicy{ + /*maxAttempts*/ 3, + /*backoffMs*/ 500, + /*retryOn*/ std::vector{"timeout", "5xx"}, + }; + auto schedule = ScheduleConfig{ + /*startAtMs*/ 1000, + /*intervalMs*/ 15000, + /*jitter*/ std::optional(250), + }; + + auto scheduleRes = ctx->scheduleAsync(job, retry, schedule).get(); + if (scheduleRes.isErr()) { + std::cerr << "Error: " << scheduleRes.error() << "\n"; + return 1; + } + std::cout << "[6] Schedule (3 complex params): jobId=" << scheduleRes->jobId + << ", willRunCount=" << scheduleRes->willRunCount + << ", firstRunAtMs=" << scheduleRes->firstRunAtMs + << ", effectiveBackoffMs=" << scheduleRes->effectiveBackoffMs + << "\n"; + + // Each `{.ffiEvent.}` declared on the Nim side gets a typed + // registration method — `addOnEchoFiredListener(handler)` here. + // A second `addEventListener` overload registers a catch-all + // wildcard listener that receives every event as raw envelope + // bytes plus the FFI return code. Both fire from the lib's + // dispatch thread, so synchronise via std::promise / atomics. + std::promise echoEvtPromise; + auto echoEvtFuture = echoEvtPromise.get_future(); + const auto typedHandle = ctx->addOnEchoFiredListener( + [&](const EchoEvent& evt) { echoEvtPromise.set_value(evt); }); + + std::atomic wildcardHits{0}; + // Wildcard listener receives every event with the wire `eventId` + // pre-extracted plus a span view over the raw CBOR envelope + // bytes (zero-copy; valid only for the duration of this call). + // Dispatch on `eventId` and use `decodeEventPayload` to lift + // the payload into a typed value without hand-parsing CBOR. + const auto wildcardHandle = ctx->addEventListener( + [&](int retCode, const std::string& eventId, + std::span envelope) { + wildcardHits.fetch_add(1); + std::cout << "[7] wildcard event: retCode=" << retCode + << ", eventId=" << eventId + << ", envelope bytes=" << envelope.size() << "\n"; + if (retCode != 0) return; + if (eventId == "on_echo_fired") { + EchoEvent decoded{}; + if (decodeEventPayload(envelope, decoded)) { + std::cout << " decoded EchoEvent: message=" + << decoded.message + << ", echoCount=" << decoded.echoCount << "\n"; + } + } + }); + + ctx->echo(EchoRequest{"event-demo", 1}); + const auto evt = echoEvtFuture.get(); + std::cout << "[7] typed event onEchoFired: message=" << evt.message + << ", echoCount=" << evt.echoCount + << ", wildcardHits=" << wildcardHits.load() << "\n"; + + // Drop the typed listener — only the wildcard fires for the + // follow-up echo. Sleep briefly to give the lib thread time to + // deliver before we tear the ctx down. + ctx->removeEventListener(typedHandle); + ctx->echo(EchoRequest{"event-demo-after-remove", 1}); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + std::cout << "[7] after removeEventListener: wildcardHits=" + << wildcardHits.load() << "\n"; + ctx->removeEventListener(wildcardHandle); + + std::cout << "\nDone.\n"; return 0; } diff --git a/examples/timer/cpp_bindings/my_timer.hpp b/examples/timer/cpp_bindings/my_timer.hpp index 7523504..8d1fc3d 100644 --- a/examples/timer/cpp_bindings/my_timer.hpp +++ b/examples/timer/cpp_bindings/my_timer.hpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,12 +24,75 @@ #include #include #include +#include extern "C" { #include } #include #include +// ============================================================ +// Result — exception-free error channel +// ============================================================ +// The generated bindings never throw: every fallible entry point (create, +// instance methods, and their *Async futures) returns a Result. Callers +// branch on isOk()/isErr() (or the explicit bool conversion) and read +// value()/error(). This mirrors the Nim side's Result[T, string] and keeps +// us off C++23's std::expected. +#ifndef NIM_FFI_RESULT_HPP_INCLUDED +#define NIM_FFI_RESULT_HPP_INCLUDED + +template +class Result { + std::optional value_; + std::string error_; +public: + static Result ok(T value) { + Result r; + r.value_ = std::move(value); + return r; + } + static Result err(std::string message) { + Result r; + r.error_ = std::move(message); + return r; + } + bool isOk() const { return value_.has_value(); } + bool isErr() const { return !value_.has_value(); } + explicit operator bool() const { return isOk(); } + const T& value() const { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + T& value() { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + const T& operator*() const { assert(value_.has_value() && "Result::operator*() called on err Result — check isOk() first"); return *value_; } + const T* operator->() const { assert(value_.has_value() && "Result::operator->() called on err Result — check isOk() first"); return &*value_; } + T&& take() { assert(value_.has_value() && "Result::take() called on err Result — check isOk() first"); return std::move(*value_); } + const std::string& error() const { assert(!value_.has_value() && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +template <> +class Result { + bool ok_ = true; + std::string error_; +public: + static Result ok() { + Result r; + r.ok_ = true; + return r; + } + static Result err(std::string message) { + Result r; + r.ok_ = false; + r.error_ = std::move(message); + return r; + } + Result() = default; + bool isOk() const { return ok_; } + bool isErr() const { return !ok_; } + explicit operator bool() const { return isOk(); } + const std::string& error() const { assert(!ok_ && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +#endif // NIM_FFI_RESULT_HPP_INCLUDED + // ── encode_cbor overloads (primitives + containers) ───────────────────── // Per-struct encode_cbor / decode_cbor are emitted by cpp.nim next to each // generated struct; these helpers cover the leaf types they defer into. @@ -161,7 +224,7 @@ inline CborError decode_cbor(CborValue& it, std::optional& out) { // ── Public entry points ───────────────────────────────────────────────── template -inline std::vector encodeCborFFI(const T& value) { +inline Result> encodeCborFFI(const T& value) { // Start with a generous 4 KiB buffer; double on overflow until it fits. std::vector buf(4096); while (true) { @@ -171,34 +234,34 @@ inline std::vector encodeCborFFI(const T& value) { if (err == CborNoError) { const size_t used = cbor_encoder_get_buffer_size(&enc, buf.data()); buf.resize(used); - return buf; + return Result>::ok(std::move(buf)); } if (err == CborErrorOutOfMemory) { const size_t extra = cbor_encoder_get_extra_bytes_needed(&enc); buf.resize(buf.size() + (extra > 0 ? extra : buf.size())); continue; } - throw std::runtime_error(std::string("FFI CBOR encode failed: ") + - cbor_error_string(err)); + return Result>::err( + std::string("FFI CBOR encode failed: ") + cbor_error_string(err)); } } template -inline T decodeCborFFI(const std::vector& bytes) { +inline Result decodeCborFFI(const std::vector& bytes) { CborParser parser; CborValue it; CborError err = cbor_parser_init(bytes.data(), bytes.size(), 0, &parser, &it); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR parse init failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR parse init failed: ") + + cbor_error_string(err)); } T out{}; err = decode_cbor(it, out); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR decode failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR decode failed: ") + + cbor_error_string(err)); } - return out; + return Result::ok(std::move(out)); } #endif // NIM_FFI_CBOR_HELPERS_HPP_INCLUDED @@ -685,22 +748,25 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { s.cv.notify_one(); } -inline std::vector ffi_call_(std::function f, - std::chrono::milliseconds timeout) { +inline Result> ffi_call_( + std::function f, + std::chrono::milliseconds timeout) { + using Bytes = std::vector; auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); if (ret == 2) { delete cb_ref; - throw std::runtime_error("RET_MISSING_CALLBACK (internal error)"); + return Result::err("RET_MISSING_CALLBACK (internal error)"); } std::unique_lock lock(state->mtx); const bool fired = state->cv.wait_for(lock, timeout, [&]{ return state->done; }); if (!fired) - throw std::runtime_error("FFI call timed out after " + std::to_string(timeout.count()) + "ms"); + return Result::err("FFI call timed out after " + + std::to_string(timeout.count()) + "ms"); if (!state->ok) - throw std::runtime_error(state->err); - return state->bytes; + return Result::err(state->err); + return Result::ok(std::move(state->bytes)); } } // anonymous namespace @@ -726,23 +792,30 @@ inline bool decodeEventPayload(std::span envelope, T& out) { class MyTimerCtx { public: - static std::unique_ptr create(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { + static Result> create(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { const auto ffi_req_ = MyTimerCreateCtorReq{config}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result>::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { (void)my_timer_create(ffi_req_bytes_.data(), ffi_req_bytes_.size(), cb, ud); return 0; }, timeout); - const auto addr_str = decodeCborFFI(ffi_raw_); - try { - const auto addr = std::stoull(addr_str); - return std::unique_ptr(new MyTimerCtx(reinterpret_cast(static_cast(addr)), timeout)); - } catch (const std::exception&) { - throw std::runtime_error("FFI create returned non-numeric address: " + addr_str); + if (ffi_raw_.isErr()) return Result>::err(ffi_raw_.error()); + auto ffi_addr_ = decodeCborFFI(ffi_raw_.value()); + if (ffi_addr_.isErr()) return Result>::err(ffi_addr_.error()); + const auto& addr_str = ffi_addr_.value(); + std::uint64_t addr = 0; + const char* addr_begin = addr_str.data(); + const char* addr_end = addr_begin + addr_str.size(); + const auto fc_ = std::from_chars(addr_begin, addr_end, addr); + if (fc_.ec != std::errc() || fc_.ptr != addr_end) { + return Result>::err("FFI create returned non-numeric address: " + addr_str); } + return Result>::ok(std::unique_ptr(new MyTimerCtx(reinterpret_cast(static_cast(addr)), timeout))); } - static std::future> createAsync(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { + static std::future>> createAsync(const TimerConfig& config, std::chrono::milliseconds timeout = std::chrono::seconds{30}) { return std::async(std::launch::async, [config, timeout]() { return create(config, timeout); }); } @@ -797,55 +870,67 @@ public: return rc == 0; } - EchoResponse echo(const EchoRequest& req) const { + Result echo(const EchoRequest& req) const { const auto ffi_req_ = MyTimerEchoReq{req}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return my_timer_echo(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future echoAsync(const EchoRequest& req) const { + std::future> echoAsync(const EchoRequest& req) const { return std::async(std::launch::async, [this, req]() { return this->echo(req); }); } - std::string version() const { + Result version() const { const auto ffi_req_ = MyTimerVersionReq{}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return my_timer_version(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future versionAsync() const { + std::future> versionAsync() const { return std::async(std::launch::async, [this]() { return this->version(); }); } - ComplexResponse complex(const ComplexRequest& req) const { + Result complex(const ComplexRequest& req) const { const auto ffi_req_ = MyTimerComplexReq{req}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return my_timer_complex(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future complexAsync(const ComplexRequest& req) const { + std::future> complexAsync(const ComplexRequest& req) const { return std::async(std::launch::async, [this, req]() { return this->complex(req); }); } - ScheduleResult schedule(const JobSpec& job, const RetryPolicy& retry, const ScheduleConfig& schedule) const { + Result schedule(const JobSpec& job, const RetryPolicy& retry, const ScheduleConfig& schedule) const { const auto ffi_req_ = MyTimerScheduleReq{job, retry, schedule}; - const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_); - const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { + auto ffi_enc_ = encodeCborFFI(ffi_req_); + if (ffi_enc_.isErr()) return Result::err(ffi_enc_.error()); + const auto& ffi_req_bytes_ = ffi_enc_.value(); + auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) { return my_timer_schedule(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size()); }, timeout_); - return decodeCborFFI(ffi_raw_); + if (ffi_raw_.isErr()) return Result::err(ffi_raw_.error()); + return decodeCborFFI(ffi_raw_.value()); } - std::future scheduleAsync(const JobSpec& job, const RetryPolicy& retry, const ScheduleConfig& schedule) const { + std::future> scheduleAsync(const JobSpec& job, const RetryPolicy& retry, const ScheduleConfig& schedule) const { return std::async(std::launch::async, [this, job, retry, schedule]() { return this->schedule(job, retry, schedule); }); } diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index 32100b2..467b26b 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -15,6 +15,7 @@ const CppPtrType* = "uint64_t" ## reflected in the generated bindings without touching this codegen. const HeaderPreludeTpl = staticRead("templates/cpp/header_prelude.hpp.tpl") + ResultTpl = staticRead("templates/cpp/result.hpp.tpl") CborHelpersTpl = staticRead("templates/cpp/cbor_helpers.hpp.tpl") SyncCallHelperTpl = staticRead("templates/cpp/sync_call_helper.hpp.tpl") ContextRuleOf5Tpl = staticRead("templates/cpp/context_rule_of_5.hpp.tpl") @@ -339,6 +340,11 @@ proc generateCppHeader*( lines.add("#include ") lines.add("#include ") + # Result is the exception-free return channel used by every generated + # entry point. It must precede the CBOR helpers and sync-call helper below, + # which now hand their failures back as Result rather than throwing. + lines.add(ResultTpl) + # CBOR primitive / container helpers must precede the per-struct codecs # below, because each emitted `encode_cbor`/`decode_cbor(T)` calls the # generic overloads for the struct's fields (std::string, std::vector, @@ -519,32 +525,43 @@ proc generateCppHeader*( # context owns library threads, so we forbid copy/move on the class # itself (see ContextRuleOf5Tpl) and hand out ownership through a # smart pointer that callers can move, store in containers, etc. + let createRet = "Result>" % [ctxTypeName] lines.add( - " static std::unique_ptr<$1> create($2) {" % - [ctxTypeName, ctorParamsWithTimeout] + " static $1 create($2) {" % [createRet, ctorParamsWithTimeout] ) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) - lines.add(" const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_);") - lines.add(" const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") + lines.add(" auto ffi_enc_ = encodeCborFFI(ffi_req_);") + lines.add(" if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [createRet]) + lines.add(" const auto& ffi_req_bytes_ = ffi_enc_.value();") + lines.add(" auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") lines.add( " (void)$1(ffi_req_bytes_.data(), ffi_req_bytes_.size(), cb, ud);" % [ctor.procName] ) lines.add(" return 0;") lines.add(" }, timeout);") - lines.add(" const auto addr_str = decodeCborFFI(ffi_raw_);") - lines.add(" try {") - lines.add(" const auto addr = std::stoull(addr_str);") - # Use `new` directly (not std::make_unique) so the ctor can stay private. + lines.add(" if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [createRet]) + lines.add(" auto ffi_addr_ = decodeCborFFI(ffi_raw_.value());") + lines.add(" if (ffi_addr_.isErr()) return $1::err(ffi_addr_.error());" % [createRet]) + lines.add(" const auto& addr_str = ffi_addr_.value();") + # Parse the ctx address without exceptions: std::stoull would throw on a + # non-numeric payload, so use std::from_chars and surface the failure as + # an err() Result instead. + lines.add(" std::uint64_t addr = 0;") + lines.add(" const char* addr_begin = addr_str.data();") + lines.add(" const char* addr_end = addr_begin + addr_str.size();") + lines.add(" const auto fc_ = std::from_chars(addr_begin, addr_end, addr);") + lines.add(" if (fc_.ec != std::errc() || fc_.ptr != addr_end) {") lines.add( - " return std::unique_ptr<$1>(new $1(reinterpret_cast(static_cast(addr)), timeout));" % - [ctxTypeName] - ) - lines.add(" } catch (const std::exception&) {") - lines.add( - " throw std::runtime_error(\"FFI create returned non-numeric address: \" + addr_str);" + " return $1::err(\"FFI create returned non-numeric address: \" + addr_str);" % + [createRet] ) lines.add(" }") + # Use `new` directly (not std::make_unique) so the ctor can stay private. + lines.add( + " return $1::ok(std::unique_ptr<$2>(new $2(reinterpret_cast(static_cast(addr)), timeout)));" % + [createRet, ctxTypeName] + ) lines.add(" }") lines.add("") @@ -559,7 +576,7 @@ proc generateCppHeader*( else: "timeout" lines.add( - " static std::future> createAsync($2) {" % + " static std::future>> createAsync($2) {" % [ctxTypeName, ctorParamsWithTimeout] ) lines.add( @@ -602,18 +619,22 @@ proc generateCppHeader*( let reqInit = cppBracedInit(reqName, methParamNames) + let methRet = "Result<$1>" % [retCppType] # Use a single-underscore-suffixed local for the Req envelope so it can't # shadow a method parameter whose name happens to be `req` (or similar). - lines.add(" $1 $2($3) const {" % [retCppType, methodName, methParamsStr]) + lines.add(" $1 $2($3) const {" % [methRet, methodName, methParamsStr]) lines.add(" const auto ffi_req_ = $1;" % [reqInit]) - lines.add(" const auto ffi_req_bytes_ = encodeCborFFI(ffi_req_);") - lines.add(" const auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") + lines.add(" auto ffi_enc_ = encodeCborFFI(ffi_req_);") + lines.add(" if (ffi_enc_.isErr()) return $1::err(ffi_enc_.error());" % [methRet]) + lines.add(" const auto& ffi_req_bytes_ = ffi_enc_.value();") + lines.add(" auto ffi_raw_ = ffi_call_([&](FFICallback cb, void* ud) {") lines.add( " return $1(ptr_, cb, ud, ffi_req_bytes_.data(), ffi_req_bytes_.size());" % [m.procName] ) lines.add(" }, timeout_);") - lines.add(" return decodeCborFFI<$1>(ffi_raw_);" % [retCppType]) + lines.add(" if (ffi_raw_.isErr()) return $1::err(ffi_raw_.error());" % [methRet]) + lines.add(" return decodeCborFFI<$1>(ffi_raw_.value());" % [retCppType]) lines.add(" }") lines.add("") # The async wrapper calls the sync method via `this->methodName(...)` so @@ -623,7 +644,7 @@ proc generateCppHeader*( if methParamsStr.len > 0: lines.add( " std::future<$1> $2Async($3) const {" % - [retCppType, methodName, methParamsStr] + [methRet, methodName, methParamsStr] ) lines.add( " return std::async(std::launch::async, [this, $1]() { return this->$2($3); });" % @@ -631,7 +652,7 @@ proc generateCppHeader*( ) lines.add(" }") else: - lines.add(" std::future<$1> $2Async() const {" % [retCppType, methodName]) + lines.add(" std::future<$1> $2Async() const {" % [methRet, methodName]) lines.add( " return std::async(std::launch::async, [this]() { return this->$1(); });" % [methodName] diff --git a/ffi/codegen/templates/cpp/cbor_helpers.hpp.tpl b/ffi/codegen/templates/cpp/cbor_helpers.hpp.tpl index a3a3574..595cc17 100644 --- a/ffi/codegen/templates/cpp/cbor_helpers.hpp.tpl +++ b/ffi/codegen/templates/cpp/cbor_helpers.hpp.tpl @@ -129,7 +129,7 @@ inline CborError decode_cbor(CborValue& it, std::optional& out) { // ── Public entry points ───────────────────────────────────────────────── template -inline std::vector encodeCborFFI(const T& value) { +inline Result> encodeCborFFI(const T& value) { // Start with a generous 4 KiB buffer; double on overflow until it fits. std::vector buf(4096); while (true) { @@ -139,34 +139,34 @@ inline std::vector encodeCborFFI(const T& value) { if (err == CborNoError) { const size_t used = cbor_encoder_get_buffer_size(&enc, buf.data()); buf.resize(used); - return buf; + return Result>::ok(std::move(buf)); } if (err == CborErrorOutOfMemory) { const size_t extra = cbor_encoder_get_extra_bytes_needed(&enc); buf.resize(buf.size() + (extra > 0 ? extra : buf.size())); continue; } - throw std::runtime_error(std::string("FFI CBOR encode failed: ") + - cbor_error_string(err)); + return Result>::err( + std::string("FFI CBOR encode failed: ") + cbor_error_string(err)); } } template -inline T decodeCborFFI(const std::vector& bytes) { +inline Result decodeCborFFI(const std::vector& bytes) { CborParser parser; CborValue it; CborError err = cbor_parser_init(bytes.data(), bytes.size(), 0, &parser, &it); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR parse init failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR parse init failed: ") + + cbor_error_string(err)); } T out{}; err = decode_cbor(it, out); if (err != CborNoError) { - throw std::runtime_error(std::string("FFI CBOR decode failed: ") + - cbor_error_string(err)); + return Result::err(std::string("FFI CBOR decode failed: ") + + cbor_error_string(err)); } - return out; + return Result::ok(std::move(out)); } #endif // NIM_FFI_CBOR_HELPERS_HPP_INCLUDED diff --git a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl index 99ff4a9..6180dd4 100644 --- a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl +++ b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,6 +24,7 @@ #include #include #include +#include extern "C" { #include } diff --git a/ffi/codegen/templates/cpp/result.hpp.tpl b/ffi/codegen/templates/cpp/result.hpp.tpl new file mode 100644 index 0000000..d58884c --- /dev/null +++ b/ffi/codegen/templates/cpp/result.hpp.tpl @@ -0,0 +1,61 @@ +// ============================================================ +// Result — exception-free error channel +// ============================================================ +// The generated bindings never throw: every fallible entry point (create, +// instance methods, and their *Async futures) returns a Result. Callers +// branch on isOk()/isErr() (or the explicit bool conversion) and read +// value()/error(). This mirrors the Nim side's Result[T, string] and keeps +// us off C++23's std::expected. +#ifndef NIM_FFI_RESULT_HPP_INCLUDED +#define NIM_FFI_RESULT_HPP_INCLUDED + +template +class Result { + std::optional value_; + std::string error_; +public: + static Result ok(T value) { + Result r; + r.value_ = std::move(value); + return r; + } + static Result err(std::string message) { + Result r; + r.error_ = std::move(message); + return r; + } + bool isOk() const { return value_.has_value(); } + bool isErr() const { return !value_.has_value(); } + explicit operator bool() const { return isOk(); } + const T& value() const { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + T& value() { assert(value_.has_value() && "Result::value() called on err Result — check isOk() first"); return *value_; } + const T& operator*() const { assert(value_.has_value() && "Result::operator*() called on err Result — check isOk() first"); return *value_; } + const T* operator->() const { assert(value_.has_value() && "Result::operator->() called on err Result — check isOk() first"); return &*value_; } + T&& take() { assert(value_.has_value() && "Result::take() called on err Result — check isOk() first"); return std::move(*value_); } + const std::string& error() const { assert(!value_.has_value() && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +template <> +class Result { + bool ok_ = true; + std::string error_; +public: + static Result ok() { + Result r; + r.ok_ = true; + return r; + } + static Result err(std::string message) { + Result r; + r.ok_ = false; + r.error_ = std::move(message); + return r; + } + Result() = default; + bool isOk() const { return ok_; } + bool isErr() const { return !ok_; } + explicit operator bool() const { return isOk(); } + const std::string& error() const { assert(!ok_ && "Result::error() called on ok Result — check isErr() first"); return error_; } +}; + +#endif // NIM_FFI_RESULT_HPP_INCLUDED diff --git a/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl b/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl index affd5f8..bfe833a 100644 --- a/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl +++ b/ffi/codegen/templates/cpp/sync_call_helper.hpp.tpl @@ -34,22 +34,25 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) { s.cv.notify_one(); } -inline std::vector ffi_call_(std::function f, - std::chrono::milliseconds timeout) { +inline Result> ffi_call_( + std::function f, + std::chrono::milliseconds timeout) { + using Bytes = std::vector; auto state = std::make_shared(); auto* cb_ref = new std::shared_ptr(state); const int ret = f(ffi_cb_, cb_ref); if (ret == 2) { delete cb_ref; - throw std::runtime_error("RET_MISSING_CALLBACK (internal error)"); + return Result::err("RET_MISSING_CALLBACK (internal error)"); } std::unique_lock lock(state->mtx); const bool fired = state->cv.wait_for(lock, timeout, [&]{ return state->done; }); if (!fired) - throw std::runtime_error("FFI call timed out after " + std::to_string(timeout.count()) + "ms"); + return Result::err("FFI call timed out after " + + std::to_string(timeout.count()) + "ms"); if (!state->ok) - throw std::runtime_error(state->err); - return state->bytes; + return Result::err(state->err); + return Result::ok(std::move(state->bytes)); } } // anonymous namespace diff --git a/tests/e2e/cpp/test_timer_e2e.cpp b/tests/e2e/cpp/test_timer_e2e.cpp index 83eeca9..a181d65 100644 --- a/tests/e2e/cpp/test_timer_e2e.cpp +++ b/tests/e2e/cpp/test_timer_e2e.cpp @@ -7,6 +7,11 @@ // genbindings_cpp` is callable end-to-end from C++. // The CrossLibrary test also loads `examples/echo/cpp_bindings` to prove // two nim-ffi libraries can coexist in one process. +// +// The generated bindings never throw: every call returns a Result. The +// `mustOk` helper below unwraps a Result and fails the test (without +// aborting) when it carries an error, so single-threaded tests read as if +// the value came back directly. #include "my_timer.hpp" #include "echo.hpp" @@ -23,8 +28,20 @@ namespace { +// Unwrap a Result in a single-threaded test context. On error it records a +// non-fatal gtest failure and returns a default-constructed T so the caller +// can keep going (subsequent expectations will fail loudly). +template +T mustOk(Result r) { + if (r.isErr()) { + ADD_FAILURE() << "unexpected FFI error: " << r.error() << " line: " << __LINE__; + return T{}; + } + return r.take(); +} + std::unique_ptr makeCtx(const std::string& name = "e2e") { - return MyTimerCtx::create(TimerConfig{name}); + return mustOk(MyTimerCtx::create(TimerConfig{name})); } } // namespace @@ -38,19 +55,19 @@ TEST(TimerE2E, CreateAndDestroy) { TEST(TimerE2E, VersionSync) { auto ctx = makeCtx("version-sync"); - const auto v = ctx->version(); + const auto v = mustOk(ctx->version()); EXPECT_EQ(v, "nim-timer v0.1.0"); } TEST(TimerE2E, VersionAsync) { auto ctx = makeCtx("version-async"); auto fut = ctx->versionAsync(); - EXPECT_EQ(fut.get(), "nim-timer v0.1.0"); + EXPECT_EQ(mustOk(fut.get()), "nim-timer v0.1.0"); } TEST(TimerE2E, EchoRoundTripsMessageAndTimerName) { auto ctx = makeCtx("echo-ctx"); - const auto resp = ctx->echo(EchoRequest{"hello", 10}); + const auto resp = mustOk(ctx->echo(EchoRequest{"hello", 10})); EXPECT_EQ(resp.echoed, "hello"); EXPECT_EQ(resp.timerName, "echo-ctx"); } @@ -60,7 +77,7 @@ TEST(TimerE2E, EchoHonoursDelay) { constexpr int delayMs = 150; const auto start = std::chrono::steady_clock::now(); - const auto resp = ctx->echo(EchoRequest{"waited", delayMs}); + const auto resp = mustOk(ctx->echo(EchoRequest{"waited", delayMs})); const auto elapsed = std::chrono::duration_cast( std::chrono::steady_clock::now() - start).count(); @@ -76,9 +93,9 @@ TEST(TimerE2E, ConcurrentAsyncCallsAreIndependent) { auto f2 = ctx->echoAsync(EchoRequest{"two", 40}); auto f3 = ctx->echoAsync(EchoRequest{"three", 20}); - const auto r3 = f3.get(); - const auto r2 = f2.get(); - const auto r1 = f1.get(); + const auto r3 = mustOk(f3.get()); + const auto r2 = mustOk(f2.get()); + const auto r1 = mustOk(f1.get()); EXPECT_EQ(r1.echoed, "one"); EXPECT_EQ(r2.echoed, "two"); @@ -97,7 +114,7 @@ TEST(TimerE2E, ComplexWithOptionalNotePresent) { std::optional(2), }; - const auto resp = ctx->complex(req); + const auto resp = mustOk(ctx->complex(req)); EXPECT_EQ(resp.itemCount, 2); EXPECT_TRUE(resp.hasNote); EXPECT_NE(resp.summary.find("note=a note"), std::string::npos) @@ -115,7 +132,7 @@ TEST(TimerE2E, ComplexWithOptionalNoteAbsent) { std::nullopt, }; - const auto resp = ctx->complex(req); + const auto resp = mustOk(ctx->complex(req)); EXPECT_EQ(resp.itemCount, 0); EXPECT_FALSE(resp.hasNote); EXPECT_NE(resp.summary.find("note="), std::string::npos) @@ -128,8 +145,8 @@ TEST(TimerE2E, IndependentContextsKeepTheirOwnState) { auto ctxA = makeCtx("alpha"); auto ctxB = makeCtx("beta"); - const auto rA = ctxA->echo(EchoRequest{"x", 5}); - const auto rB = ctxB->echo(EchoRequest{"x", 5}); + const auto rA = mustOk(ctxA->echo(EchoRequest{"x", 5})); + const auto rB = mustOk(ctxB->echo(EchoRequest{"x", 5})); EXPECT_EQ(rA.timerName, "alpha"); EXPECT_EQ(rB.timerName, "beta"); @@ -137,8 +154,8 @@ TEST(TimerE2E, IndependentContextsKeepTheirOwnState) { // N contexts keep independent state; an error on one must not poison siblings. // Empty JobSpec.name is the chosen error trigger: schedule() returns -// err("job name must not be empty"), which the bindings rethrow as -// std::runtime_error carrying the exact string. +// err("job name must not be empty"), which the bindings surface as an +// err() Result carrying the exact string. TEST(TimerE2E, MultiContextIsolation) { constexpr int kCtxCount = 5; std::vector> ctxs; @@ -148,7 +165,7 @@ TEST(TimerE2E, MultiContextIsolation) { } for (int i = 0; i < kCtxCount; ++i) { - const auto resp = ctxs[i]->echo(EchoRequest{"ping", 0}); + const auto resp = mustOk(ctxs[i]->echo(EchoRequest{"ping", 0})); EXPECT_EQ(resp.echoed, "ping"); EXPECT_EQ(resp.timerName, "iso-" + std::to_string(i)); } @@ -156,20 +173,17 @@ TEST(TimerE2E, MultiContextIsolation) { const auto bad = JobSpec{/*name*/ "", /*payload*/ {}, /*priority*/ 0}; const auto retry = RetryPolicy{1, 10, {}}; const auto sched = ScheduleConfig{0, 0, std::nullopt}; - try { - (void)ctxs[2]->schedule(bad, retry, sched); - FAIL() << "expected schedule() to throw on empty job name"; - } catch (const std::runtime_error& ex) { - EXPECT_STREQ(ex.what(), "job name must not be empty"); - } + const auto scheduleRes = ctxs[2]->schedule(bad, retry, sched); + ASSERT_TRUE(scheduleRes.isErr()) << "expected schedule() to fail on empty job name"; + EXPECT_EQ(scheduleRes.error(), "job name must not be empty"); - const auto recovered = ctxs[2]->echo(EchoRequest{"after-err", 0}); + const auto recovered = mustOk(ctxs[2]->echo(EchoRequest{"after-err", 0})); EXPECT_EQ(recovered.echoed, "after-err"); EXPECT_EQ(recovered.timerName, "iso-2"); for (int i = 0; i < kCtxCount; ++i) { if (i == 2) continue; - const auto resp = ctxs[i]->echo(EchoRequest{"still-here", 0}); + const auto resp = mustOk(ctxs[i]->echo(EchoRequest{"still-here", 0})); EXPECT_EQ(resp.echoed, "still-here"); EXPECT_EQ(resp.timerName, "iso-" + std::to_string(i)); } @@ -177,31 +191,31 @@ TEST(TimerE2E, MultiContextIsolation) { // Two nim-ffi libraries in one process must not share state or symbols. TEST(TimerE2E, CrossLibrary) { - auto timerCtx = MyTimerCtx::create(TimerConfig{"x-timer"}); - auto echoCtx = EchoCtx::create(EchoConfig{"X-ECHO"}); + auto timerCtx = mustOk(MyTimerCtx::create(TimerConfig{"x-timer"})); + auto echoCtx = mustOk(EchoCtx::create(EchoConfig{"X-ECHO"})); - EXPECT_EQ(timerCtx->version(), "nim-timer v0.1.0"); - EXPECT_EQ(echoCtx->version(), "nim-echo v0.1.0"); + EXPECT_EQ(mustOk(timerCtx->version()), "nim-timer v0.1.0"); + EXPECT_EQ(mustOk(echoCtx->version()), "nim-echo v0.1.0"); - const auto timerResp = timerCtx->echo(EchoRequest{"hello", 0}); + const auto timerResp = mustOk(timerCtx->echo(EchoRequest{"hello", 0})); EXPECT_EQ(timerResp.echoed, "hello"); EXPECT_EQ(timerResp.timerName, "x-timer"); - const auto echoResp = echoCtx->shout(ShoutRequest{"hello"}); + const auto echoResp = mustOk(echoCtx->shout(ShoutRequest{"hello"})); EXPECT_EQ(echoResp.shouted, "X-ECHO: HELLO"); EXPECT_EQ(echoResp.prefix, "X-ECHO"); for (int i = 0; i < 4; ++i) { - const auto t = timerCtx->echo(EchoRequest{"t" + std::to_string(i), 0}); - const auto e = echoCtx->shout(ShoutRequest{"e" + std::to_string(i)}); + const auto t = mustOk(timerCtx->echo(EchoRequest{"t" + std::to_string(i), 0})); + const auto e = mustOk(echoCtx->shout(ShoutRequest{"e" + std::to_string(i)})); EXPECT_EQ(t.timerName, "x-timer"); EXPECT_EQ(e.prefix, "X-ECHO"); } auto tFut = timerCtx->echoAsync(EchoRequest{"async-t", 30}); auto eFut = echoCtx->shoutAsync(ShoutRequest{"async-e"}); - const auto t = tFut.get(); - const auto e = eFut.get(); + const auto t = mustOk(tFut.get()); + const auto e = mustOk(eFut.get()); EXPECT_EQ(t.echoed, "async-t"); EXPECT_EQ(t.timerName, "x-timer"); EXPECT_EQ(e.shouted, "X-ECHO: ASYNC-E"); @@ -212,9 +226,9 @@ TEST(TimerE2E, TriplePipeline) { auto ctx = makeCtx("pipeline"); auto pipeline = std::async(std::launch::async, [&ctx]() { - auto a = ctx->echoAsync(EchoRequest{"A", 20}).get(); - auto b = ctx->echoAsync(EchoRequest{a.echoed + "->B", 10}).get(); - auto c = ctx->echoAsync(EchoRequest{b.echoed + "->C", 5}).get(); + auto a = mustOk(ctx->echoAsync(EchoRequest{"A", 20}).get()); + auto b = mustOk(ctx->echoAsync(EchoRequest{a.echoed + "->B", 10}).get()); + auto c = mustOk(ctx->echoAsync(EchoRequest{b.echoed + "->C", 5}).get()); return c; }); @@ -224,6 +238,8 @@ TEST(TimerE2E, TriplePipeline) { } // Per-thread context create -> one call -> destroy churns the FFI context pool. +// Worker threads avoid gtest assertion macros (not thread-safe) and report via +// the atomic `errors` counter instead. TEST(TimerE2E, StressShortLivedPerThreadContext) { constexpr int kThreads = 16; @@ -233,14 +249,13 @@ TEST(TimerE2E, StressShortLivedPerThreadContext) { for (int t = 0; t < kThreads; ++t) { workers.emplace_back([&, t] { - try { - auto ctx = makeCtx("short-" + std::to_string(t)); - const auto resp = ctx->echo(EchoRequest{"hi", 0}); - if (resp.echoed != "hi") ++errors; - if (resp.timerName != "short-" + std::to_string(t)) ++errors; - } catch (const std::exception&) { - ++errors; - } + auto ctxRes = MyTimerCtx::create(TimerConfig{"short-" + std::to_string(t)}); + if (ctxRes.isErr()) { ++errors; return; } + auto ctx = std::move(ctxRes.value()); + const auto resp = ctx->echo(EchoRequest{"hi", 0}); + if (resp.isErr()) { ++errors; return; } + if (resp->echoed != "hi") ++errors; + if (resp->timerName != "short-" + std::to_string(t)) ++errors; }); } for (auto& w : workers) w.join(); @@ -259,13 +274,10 @@ TEST(TimerE2E, StressShortLivedSharedContext) { for (int t = 0; t < kThreads; ++t) { workers.emplace_back([&, t] { - try { - const auto resp = shared->echo(EchoRequest{"x" + std::to_string(t), 0}); - if (resp.echoed != "x" + std::to_string(t)) ++errors; - if (resp.timerName != "shared-short") ++errors; - } catch (const std::exception&) { - ++errors; - } + const auto resp = shared->echo(EchoRequest{"x" + std::to_string(t), 0}); + if (resp.isErr()) { ++errors; return; } + if (resp->echoed != "x" + std::to_string(t)) ++errors; + if (resp->timerName != "shared-short") ++errors; }); } for (auto& w : workers) w.join(); @@ -289,14 +301,17 @@ TEST(TimerE2E, ThreadedHammer) { for (int t = 0; t < kThreads; ++t) { workers.emplace_back([&, t] { - auto own = makeCtx("hammer-t" + std::to_string(t)); + auto ownRes = MyTimerCtx::create(TimerConfig{"hammer-t" + std::to_string(t)}); + if (ownRes.isErr()) { ++errors; return; } + auto own = std::move(ownRes.value()); for (int i = 0; i < kIters; ++i) { if ((i & 1) == 0) { const auto r = shared->echo(EchoRequest{"s", 0}); - if (r.echoed != "s") ++errors; + if (r.isErr() || r->echoed != "s") ++errors; } else { auto f = own->echoAsync(EchoRequest{"a", 1}); - if (f.get().echoed != "a") ++errors; + const auto r = f.get(); + if (r.isErr() || r->echoed != "a") ++errors; } } }); @@ -322,7 +337,7 @@ TEST(TimerE2E, TypedEventFiresAfterEcho) { [&](const EchoEvent& evt) { evtPromise.set_value(evt); }); ASSERT_NE(handle.id, 0u) << "addOnEchoFiredListener returned zero id"; - const auto resp = ctx->echo(EchoRequest{"event-msg", 1}); + const auto resp = mustOk(ctx->echo(EchoRequest{"event-msg", 1})); EXPECT_EQ(resp.echoed, "event-msg"); const auto status = evtFuture.wait_for(std::chrono::seconds(2)); From c43563f82f364e4249858c4397a8967fca2f5b7d Mon Sep 17 00:00:00 2001 From: Ivan FB <128452529+Ivansete-status@users.noreply.github.com> Date: Fri, 29 May 2026 20:40:28 +0200 Subject: [PATCH 2/4] rust codegen: per-event typed add_on__listener + wildcard add_event_listener (#52) --- examples/timer/rust_bindings/Cargo.lock | 20 +- examples/timer/rust_bindings/src/api.rs | 166 ++++++++----- examples/timer/rust_client/Cargo.lock | 22 +- ffi/codegen/rust.nim | 309 ++++++++++++++++-------- 4 files changed, 328 insertions(+), 189 deletions(-) diff --git a/examples/timer/rust_bindings/Cargo.lock b/examples/timer/rust_bindings/Cargo.lock index 9e6f26d..73e8c29 100644 --- a/examples/timer/rust_bindings/Cargo.lock +++ b/examples/timer/rust_bindings/Cargo.lock @@ -84,6 +84,16 @@ dependencies = [ "scopeguard", ] +[[package]] +name = "my_timer" +version = "0.1.0" +dependencies = [ + "ciborium", + "flume", + "serde", + "tokio", +] + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -164,16 +174,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "timer" -version = "0.1.0" -dependencies = [ - "ciborium", - "flume", - "serde", - "tokio", -] - [[package]] name = "tokio" version = "1.52.3" diff --git a/examples/timer/rust_bindings/src/api.rs b/examples/timer/rust_bindings/src/api.rs index 6794d37..d8d9721 100644 --- a/examples/timer/rust_bindings/src/api.rs +++ b/examples/timer/rust_bindings/src/api.rs @@ -98,63 +98,72 @@ where } } -/// Typed event handlers for `MyTimerCtx`. Each field is `None` by -/// default; set the ones you care about and pass to -/// `MyTimerCtx::set_event_handlers`. -#[allow(non_snake_case)] -pub struct Events { - pub on_error: Option>, - pub onEchoFired: Option>, +struct OnEchoFiredHandler { + f: Box, } -impl Default for Events { - fn default() -> Self { - Self { on_error: None, onEchoFired: None } +unsafe extern "C" fn on_echo_fired_trampoline( + ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void, +) { + if ud.is_null() || ret != 0 || msg.is_null() || len == 0 { + return; + } + let h = &*(ud as *const OnEchoFiredHandler); + let bytes = slice::from_raw_parts(msg as *const u8, len); + #[derive(serde::Deserialize)] + struct Envelope { payload: EchoEvent } + if let Ok(env) = ciborium::de::from_reader::(bytes) { + (h.f)(&env.payload); } } -unsafe extern "C" fn my_timer_event_trampoline( +struct WildcardHandler { + f: Box, +} + +unsafe extern "C" fn my_timer_wildcard_trampoline( ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void, ) { if ud.is_null() { return; } - let events = &*(ud as *const Events); - if ret != 0 { - if let Some(ref on_err) = events.on_error { - let bytes = if !msg.is_null() && len > 0 { - slice::from_raw_parts(msg as *const u8, len) - } else { &[] }; - let s = String::from_utf8_lossy(bytes); - on_err(&s); - } - return; - } - if msg.is_null() || len == 0 { return; } - let bytes = slice::from_raw_parts(msg as *const u8, len); - #[derive(serde::Deserialize)] - struct EnvelopeMeta { - #[serde(rename = "eventType")] - event_type: String, - } - let meta: EnvelopeMeta = match ciborium::de::from_reader(bytes) { - Ok(m) => m, - Err(_) => return, - }; - if meta.event_type == "on_echo_fired" { + let h = &*(ud as *const WildcardHandler); + let bytes = if !msg.is_null() && len > 0 { + slice::from_raw_parts(msg as *const u8, len) + } else { &[] }; + let event_id = if ret == 0 && !bytes.is_empty() { #[derive(serde::Deserialize)] - struct Envelope { payload: EchoEvent } - if let Ok(env) = ciborium::de::from_reader::(bytes) { - if let Some(ref h) = events.onEchoFired { h(&env.payload); } + struct EnvelopeMeta { + #[serde(rename = "eventType")] + event_type: String, } - return; - } + ciborium::de::from_reader::(bytes) + .map(|m| m.event_type).unwrap_or_default() + } else { + String::new() + }; + (h.f)(ret, event_id.as_str(), bytes); +} + +#[derive(Debug, Clone, Copy)] +pub struct ListenerHandle { pub id: u64 } + +/// Decode the `payload` field of a CBOR `EventEnvelope` as `T`. +/// Returns `Err` if the envelope is empty / malformed / the payload +/// cannot be deserialised as `T`. +pub fn decode_event_payload( + envelope: &[u8], +) -> Result { + #[derive(serde::Deserialize)] + struct Envelope { payload: T } + let env: Envelope = ciborium::de::from_reader(envelope) + .map_err(|e| format!("decode event payload: {e}"))?; + Ok(env.payload) } /// High-level context for `MyTimer`. pub struct MyTimerCtx { ptr: *mut c_void, timeout: Duration, - events: *mut Events, - event_listener_id: u64, + listeners: std::sync::Mutex>>, } // SAFETY: The `ptr` field points to an FFIContext owned by the Nim runtime. @@ -174,10 +183,6 @@ impl Drop for MyTimerCtx { unsafe { ffi::my_timer_destroy(self.ptr); } self.ptr = std::ptr::null_mut(); } - if !self.events.is_null() { - unsafe { drop(Box::from_raw(self.events)); } - self.events = std::ptr::null_mut(); - } } } @@ -191,7 +196,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(), event_listener_id: 0 }) + Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) }) } pub async fn new_async(config: TimerConfig, timeout: Duration) -> Result { @@ -203,30 +208,57 @@ 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(), event_listener_id: 0 }) + Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) }) } - /// 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(); - } - let raw = Box::into_raw(Box::new(handlers)); - self.events = raw; - unsafe { - 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); + fn add_listener_inner( + &self, + event_name: *const c_char, + callback: ffi::FFICallback, + raw: *mut c_void, + owned: Box, + ) -> ListenerHandle { + let id = unsafe { + ffi::my_timer_add_event_listener(self.ptr, event_name, callback, raw) + }; + if id != 0 { + self.listeners.lock().unwrap().insert(id, owned); } + ListenerHandle { id } + } + + /// Register a typed listener for `on_echo_fired`. The returned handle can be + /// passed to `remove_event_listener` to unregister. + pub fn add_on_echo_fired_listener(&self, handler: F) -> ListenerHandle + where F: Fn(&EchoEvent) + Send + Sync + 'static, + { + let owned: Box = Box::new(OnEchoFiredHandler { f: Box::new(handler) }); + let raw = &*owned as *const OnEchoFiredHandler as *mut c_void; + self.add_listener_inner(b"on_echo_fired\0".as_ptr() as *const c_char, on_echo_fired_trampoline, raw, owned) + } + + /// Register a catch-all listener that receives every event. + /// The handler arguments are (return_code, event_id, envelope_bytes): + /// `event_id` is the wire `eventType` string extracted from the + /// envelope (empty on error or malformed envelope); `envelope_bytes` + /// is the full CBOR envelope, suitable for `decode_event_payload::`. + pub fn add_event_listener(&self, handler: F) -> ListenerHandle + where F: Fn(c_int, &str, &[u8]) + Send + Sync + 'static, + { + let owned: Box = Box::new(WildcardHandler { f: Box::new(handler) }); + let raw = &*owned as *const WildcardHandler as *mut c_void; + self.add_listener_inner(b"\0".as_ptr() as *const c_char, my_timer_wildcard_trampoline, raw, owned) + } + + /// Remove a previously-registered listener by handle. Returns true + /// if the listener existed and was removed; false otherwise. + pub fn remove_event_listener(&self, handle: ListenerHandle) -> bool { + if handle.id == 0 { return false; } + let rc = unsafe { + ffi::my_timer_remove_event_listener(self.ptr, handle.id) + }; + self.listeners.lock().unwrap().remove(&handle.id); + rc == 0 } pub fn echo(&self, req: EchoRequest) -> Result { diff --git a/examples/timer/rust_client/Cargo.lock b/examples/timer/rust_client/Cargo.lock index 9e0ca6f..6f1d949 100644 --- a/examples/timer/rust_client/Cargo.lock +++ b/examples/timer/rust_client/Cargo.lock @@ -96,6 +96,16 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "my_timer" +version = "0.1.0" +dependencies = [ + "ciborium", + "flume", + "serde", + "tokio", +] + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -124,8 +134,8 @@ dependencies = [ name = "rust_client" version = "0.1.0" dependencies = [ + "my_timer", "serde_json", - "timer", "tokio", ] @@ -198,16 +208,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "timer" -version = "0.1.0" -dependencies = [ - "ciborium", - "flume", - "serde", - "tokio", -] - [[package]] name = "tokio" version = "1.52.1" diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 20c46c0..4afbefb 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -207,8 +207,8 @@ proc generateFFIRs*(procs: seq[FFIProcMeta]): string = params.add("ctx: *mut c_void") lines.add(" pub fn $1($2) -> c_int;" % [p.procName, params.join(", ")]) - # Listener-registration ABI — emitted by `declareLibrary`, always - # present in the dylib. + # Listener-registration ABI — emitted on the Nim side by `declareLibrary`, + # always present in the dylib. lines.add( " pub fn $1_add_event_listener(ctx: *mut c_void, event_name: *const c_char, callback: FFICallback, user_data: *mut c_void) -> u64;" % [linkLibName] @@ -446,77 +446,105 @@ proc generateApiRs*( lines.add("}") lines.add("") - # ── Typed event handler struct + trampoline (only if events declared) ──── - # The Events struct holds optional boxed closures, one per registered - # `{.ffiEvent.}`. The struct lives on the heap (Box::into_raw); its raw - # pointer is handed to the dylib as `user_data` for the event callback. - # The trampoline parses the CBOR `EventEnvelope`, picks the matching - # field on Events, decodes the payload as the registered type, and - # invokes the closure. + # ── Per-listener handler boxes + extern "C" trampolines ───────────────── + # Each registered listener owns a `Box<…Handler>` that is kept alive in + # `$1::listeners` (keyed by listener id). The raw pointer to the inner + # handler is handed to the dylib as `user_data` for the per-event or + # wildcard trampoline below. if events.len > 0: - lines.add("/// Typed event handlers for `$1`. Each field is `None` by" % [ctxTypeName]) - lines.add("/// default; set the ones you care about and pass to") - lines.add("/// `$1::set_event_handlers`." % [ctxTypeName]) - lines.add("#[allow(non_snake_case)]") - lines.add("pub struct Events {") - lines.add(" pub on_error: Option>,") for ev in events: + let handlerStruct = capitalizeFirstLetter(ev.nimProcName) & "Handler" + let trampolineName = camelToSnakeCase(ev.nimProcName) & "_trampoline" + lines.add("struct $1 {" % [handlerStruct]) lines.add( - " pub $1: Option>," % - [ev.nimProcName, ev.payloadTypeName] + " f: Box," % [ev.payloadTypeName] ) - lines.add("}") - lines.add("") - lines.add("impl Default for Events {") - lines.add(" fn default() -> Self {") - lines.add(" Self { on_error: None, " & - events.mapIt(it.nimProcName & ": None").join(", ") & " }") - lines.add(" }") - lines.add("}") - lines.add("") + lines.add("}") + lines.add("") + lines.add("unsafe extern \"C\" fn $1(" % [trampolineName]) + lines.add(" ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void,") + lines.add(") {") + lines.add(" if ud.is_null() || ret != 0 || msg.is_null() || len == 0 {") + lines.add(" return;") + lines.add(" }") + lines.add(" let h = &*(ud as *const $1);" % [handlerStruct]) + lines.add(" let bytes = slice::from_raw_parts(msg as *const u8, len);") + lines.add(" #[derive(serde::Deserialize)]") + lines.add( + " struct Envelope { payload: $1 }" % [ev.payloadTypeName] + ) + lines.add( + " if let Ok(env) = ciborium::de::from_reader::(bytes) {" + ) + lines.add(" (h.f)(&env.payload);") + lines.add(" }") + lines.add("}") + lines.add("") - # Trampoline — `extern "C"` free function. Deserialises the envelope - # twice: once for `eventType`, then with the typed payload via serde. - lines.add("unsafe extern \"C\" fn $1_event_trampoline(" % [libName]) + # Wildcard handler — receives every event as raw envelope bytes, + # the FFI return code, and the `eventType` string pre-extracted + # from the CBOR envelope. `event_id` is empty when `ret != 0` or + # the envelope is malformed (the bytes are an error string, not a + # CBOR envelope, in that case). + lines.add("struct WildcardHandler {") + lines.add(" f: Box,") + lines.add("}") + lines.add("") + lines.add("unsafe extern \"C\" fn $1_wildcard_trampoline(" % [libName]) lines.add(" ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void,") lines.add(") {") lines.add(" if ud.is_null() { return; }") - lines.add(" let events = &*(ud as *const Events);") - lines.add(" if ret != 0 {") - lines.add(" if let Some(ref on_err) = events.on_error {") - lines.add(" let bytes = if !msg.is_null() && len > 0 {") - lines.add(" slice::from_raw_parts(msg as *const u8, len)") - lines.add(" } else { &[] };") - lines.add(" let s = String::from_utf8_lossy(bytes);") - lines.add(" on_err(&s);") + lines.add(" let h = &*(ud as *const WildcardHandler);") + lines.add(" let bytes = if !msg.is_null() && len > 0 {") + lines.add(" slice::from_raw_parts(msg as *const u8, len)") + lines.add(" } else { &[] };") + lines.add(" let event_id = if ret == 0 && !bytes.is_empty() {") + lines.add(" #[derive(serde::Deserialize)]") + lines.add(" struct EnvelopeMeta {") + lines.add(" #[serde(rename = \"eventType\")]") + lines.add(" event_type: String,") lines.add(" }") - lines.add(" return;") - lines.add(" }") - lines.add(" if msg.is_null() || len == 0 { return; }") - lines.add(" let bytes = slice::from_raw_parts(msg as *const u8, len);") - lines.add(" #[derive(serde::Deserialize)]") - lines.add(" struct EnvelopeMeta {") - lines.add(" #[serde(rename = \"eventType\")]") - lines.add(" event_type: String,") - lines.add(" }") - lines.add(" let meta: EnvelopeMeta = match ciborium::de::from_reader(bytes) {") - lines.add(" Ok(m) => m,") - lines.add(" Err(_) => return,") + lines.add( + " ciborium::de::from_reader::(bytes)" + ) + lines.add(" .map(|m| m.event_type).unwrap_or_default()") + lines.add(" } else {") + lines.add(" String::new()") lines.add(" };") - for ev in events: - lines.add(" if meta.event_type == \"$1\" {" % [ev.wireName]) - lines.add(" #[derive(serde::Deserialize)]") - lines.add(" struct Envelope { payload: $1 }" % [ev.payloadTypeName]) - lines.add( - " if let Ok(env) = ciborium::de::from_reader::(bytes) {" - ) - lines.add( - " if let Some(ref h) = events.$1 { h(&env.payload); }" % - [ev.nimProcName] - ) - lines.add(" }") - lines.add(" return;") - lines.add(" }") + lines.add(" (h.f)(ret, event_id.as_str(), bytes);") + lines.add("}") + lines.add("") + + # Public handle returned by every add_…_listener call. + lines.add("#[derive(Debug, Clone, Copy)]") + lines.add("pub struct ListenerHandle { pub id: u64 }") + lines.add("") + + # Helper: decode an event envelope's `payload` field into any typed + # `T` that the generated `types.rs` already derives `Deserialize` on. + # Pair with `add_event_listener` to lift raw envelope bytes into a + # typed payload without hand-rolling ciborium calls in each branch. + lines.add( + "/// Decode the `payload` field of a CBOR `EventEnvelope` as `T`." + ) + lines.add( + "/// Returns `Err` if the envelope is empty / malformed / the payload" + ) + lines.add("/// cannot be deserialised as `T`.") + lines.add( + "pub fn decode_event_payload(" + ) + lines.add(" envelope: &[u8],") + lines.add(") -> Result {") + lines.add(" #[derive(serde::Deserialize)]") + lines.add(" struct Envelope { payload: T }") + lines.add( + " let env: Envelope = ciborium::de::from_reader(envelope)" + ) + lines.add( + " .map_err(|e| format!(\"decode event payload: {e}\"))?;" + ) + lines.add(" Ok(env.payload)") lines.add("}") lines.add("") @@ -526,8 +554,14 @@ proc generateApiRs*( lines.add(" ptr: *mut c_void,") lines.add(" timeout: Duration,") if events.len > 0: - lines.add(" events: *mut Events,") - lines.add(" event_listener_id: u64,") + # Keeps each registered handler box alive while its listener id is + # live on the Nim side. Removing an entry from the map drops the + # Box and frees the user's closure; the Nim-side registry has + # already guaranteed no callback for that id is in flight by the + # time `_remove_event_listener` returns. + lines.add( + " listeners: std::sync::Mutex>>," + ) lines.add("}") lines.add("") # SAFETY block applies to both impls below (PR #23 Rust review, item 7). @@ -564,13 +598,9 @@ proc generateApiRs*( lines.add(" unsafe { ffi::$1(self.ptr); }" % [dtorProcName]) lines.add(" self.ptr = std::ptr::null_mut();") lines.add(" }") - # Reclaim the Events box after the dylib's destroy has torn down the - # FFI thread (no more events will fire by this point). - if events.len > 0: - lines.add(" if !self.events.is_null() {") - lines.add(" unsafe { drop(Box::from_raw(self.events)); }") - lines.add(" self.events = std::ptr::null_mut();") - lines.add(" }") + # `listeners` is dropped automatically after this body returns. By + # that point the dylib has joined its threads, so no callback is mid- + # flight against any of the raw pointers we handed it. lines.add(" }") lines.add("}") lines.add("") @@ -627,7 +657,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(), event_listener_id: 0 })") + lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })") else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") @@ -653,49 +683,126 @@ 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(), event_listener_id: 0 })") + lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout, listeners: std::sync::Mutex::new(std::collections::HashMap::new()) })") else: lines.add(" Ok(Self { ptr: addr as *mut c_void, timeout })") lines.add(" }") lines.add("") - # ── Typed event registration ─────────────────────────────────────────── + # ── Listener-registration API ───────────────────────────────────────── if events.len > 0: + # Private helper shared by every public `add_*_listener`: the + # FFI call + map insertion is identical across the typed and + # wildcard variants, so it lives in one place. The caller owns + # the box (typed as the concrete handler struct so the raw + # pointer matches the trampoline's expected type) and only + # erases it to `dyn Any + Send` when handing ownership over. + lines.add(" fn add_listener_inner(") + lines.add(" &self,") + lines.add(" event_name: *const c_char,") + lines.add(" callback: ffi::FFICallback,") + lines.add(" raw: *mut c_void,") + lines.add(" owned: Box,") + lines.add(" ) -> ListenerHandle {") + lines.add(" let id = unsafe {") 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);" % + " ffi::$1_add_event_listener(self.ptr, event_name, callback, raw)" % [libName] ) - lines.add(" }") - lines.add(" self.event_listener_id = 0;") + lines.add(" };") + lines.add(" if id != 0 {") + lines.add(" self.listeners.lock().unwrap().insert(id, owned);") 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();") - lines.add(" }") - lines.add(" let raw = Box::into_raw(Box::new(handlers));") - lines.add(" self.events = raw;") - lines.add(" unsafe {") + lines.add(" ListenerHandle { id }") + lines.add(" }") + lines.add("") + + for ev in events: + let methodName = "add_" & camelToSnakeCase(ev.nimProcName) & "_listener" + let handlerStruct = capitalizeFirstLetter(ev.nimProcName) & "Handler" + let trampolineName = camelToSnakeCase(ev.nimProcName) & "_trampoline" + lines.add( + " /// Register a typed listener for `$1`. The returned handle can be" % + [ev.wireName] + ) + lines.add(" /// passed to `remove_event_listener` to unregister.") + lines.add( + " pub fn $1(&self, handler: F) -> ListenerHandle" % [methodName] + ) + lines.add( + " where F: Fn(&$1) + Send + Sync + 'static," % [ev.payloadTypeName] + ) + lines.add(" {") + lines.add( + " let owned: Box<$1> = Box::new($1 { f: Box::new(handler) });" % + [handlerStruct] + ) + lines.add(" let raw = &*owned as *const $1 as *mut c_void;" % + [handlerStruct]) + lines.add( + " self.add_listener_inner(b\"$1\\0\".as_ptr() as *const c_char, $2, raw, owned)" % + [ev.wireName, trampolineName] + ) + lines.add(" }") + lines.add("") + + # Generic wildcard listener — receives every event with the wire + # `eventType` string pre-extracted plus the raw envelope bytes. Pair + # with `decode_event_payload::` to lift the payload into a typed + # value. lines.add( - " self.event_listener_id = ffi::$1_add_event_listener(" % + " /// Register a catch-all listener that receives every event." + ) + lines.add( + " /// The handler arguments are (return_code, event_id, envelope_bytes):" + ) + lines.add( + " /// `event_id` is the wire `eventType` string extracted from the" + ) + lines.add( + " /// envelope (empty on error or malformed envelope); `envelope_bytes`" + ) + lines.add( + " /// is the full CBOR envelope, suitable for `decode_event_payload::`." + ) + lines.add( + " pub fn add_event_listener(&self, handler: F) -> ListenerHandle" + ) + lines.add(" where F: Fn(c_int, &str, &[u8]) + Send + Sync + 'static,") + lines.add(" {") + lines.add( + " let owned: Box = Box::new(WildcardHandler { f: Box::new(handler) });" + ) + lines.add( + " let raw = &*owned as *const WildcardHandler as *mut c_void;" + ) + lines.add( + " self.add_listener_inner(b\"\\0\".as_ptr() as *const c_char, $1_wildcard_trampoline, raw, owned)" % [libName] ) - lines.add(" self.ptr, b\"\\0\".as_ptr() as *const c_char,") + lines.add(" }") + lines.add("") + + # Remove by handle. Drops the Box (and the user's closure) after the + # C ABI confirms the listener has been unregistered. lines.add( - " $1_event_trampoline, raw as *mut c_void);" % [libName] + " /// Remove a previously-registered listener by handle. Returns true" ) - lines.add(" }") + lines.add( + " /// if the listener existed and was removed; false otherwise." + ) + lines.add( + " pub fn remove_event_listener(&self, handle: ListenerHandle) -> bool {" + ) + lines.add(" if handle.id == 0 { return false; }") + lines.add(" let rc = unsafe {") + lines.add( + " ffi::$1_remove_event_listener(self.ptr, handle.id)" % + [libName] + ) + lines.add(" };") + lines.add(" self.listeners.lock().unwrap().remove(&handle.id);") + lines.add(" rc == 0") lines.add(" }") lines.add("") From e0bd74232b4ca23a20722eab1b1c6327c0acbd98 Mon Sep 17 00:00:00 2001 From: Ivan FB <128452529+Ivansete-status@users.noreply.github.com> Date: Mon, 1 Jun 2026 18:34:56 +0200 Subject: [PATCH 3/4] Rust event examples (#53) * rust examples: sync main.rs + tokio main.rs demoing the listener API Adds two bundled examples to the generated Rust crate: - examples/main.rs: sync flow using std::sync::mpsc to bridge a typed on_echo_fired listener into main + a wildcard add_event_listener that uses decode_event_payload::(envelope) for the matching event id. - examples/tokio_main.rs: same shape via #[tokio::main] + tokio::sync::mpsc. Bumps generateCargoToml to ship `[dev-dependencies]` with tokio's `rt-multi-thread` + `macros` features so the bundled examples can use #[tokio::main] without polluting the library's runtime profile. Run with `cargo run --example main` (set DYLD_LIBRARY_PATH= on macOS until build.rs emits an rpath). Co-Authored-By: Claude Opus 4.7 * simplify examples --------- Co-authored-by: Claude Opus 4.7 --- examples/timer/rust_bindings/Cargo.lock | 12 ++++ examples/timer/rust_bindings/Cargo.toml | 3 + examples/timer/rust_bindings/examples/main.rs | 53 ++++++++++++++++ .../rust_bindings/examples/tokio_main.rs | 60 +++++++++++++++++++ ffi/codegen/rust.nim | 5 ++ 5 files changed, 133 insertions(+) create mode 100644 examples/timer/rust_bindings/examples/main.rs create mode 100644 examples/timer/rust_bindings/examples/tokio_main.rs diff --git a/examples/timer/rust_bindings/Cargo.lock b/examples/timer/rust_bindings/Cargo.lock index 73e8c29..2ca631d 100644 --- a/examples/timer/rust_bindings/Cargo.lock +++ b/examples/timer/rust_bindings/Cargo.lock @@ -181,6 +181,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" dependencies = [ "pin-project-lite", + "tokio-macros", +] + +[[package]] +name = "tokio-macros" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" +dependencies = [ + "proc-macro2", + "quote", + "syn", ] [[package]] diff --git a/examples/timer/rust_bindings/Cargo.toml b/examples/timer/rust_bindings/Cargo.toml index b34251a..ecbb01d 100644 --- a/examples/timer/rust_bindings/Cargo.toml +++ b/examples/timer/rust_bindings/Cargo.toml @@ -8,3 +8,6 @@ serde = { version = "1", features = ["derive"] } ciborium = "0.2" flume = { version = "0.11", default-features = false, features = ["async"] } tokio = { version = "1", features = ["sync", "time"] } + +[dev-dependencies] +tokio = { version = "1", features = ["rt-multi-thread", "macros", "sync", "time"] } diff --git a/examples/timer/rust_bindings/examples/main.rs b/examples/timer/rust_bindings/examples/main.rs new file mode 100644 index 0000000..b951369 --- /dev/null +++ b/examples/timer/rust_bindings/examples/main.rs @@ -0,0 +1,53 @@ +//! Synchronous example: exercises the library-event listener API +//! (typed + wildcard + remove). +//! +//! Run with: `cargo run --example main` + +use my_timer::{decode_event_payload, EchoEvent, EchoRequest, MyTimerCtx, TimerConfig}; +use std::os::raw::c_int; +use std::sync::mpsc; +use std::time::Duration; + +fn main() -> Result<(), String> { + let ctx = MyTimerCtx::create( + TimerConfig { name: "rust-sync-demo".into() }, + Duration::from_secs(5), + )?; + + // Typed listener: the closure is invoked on the lib's dispatch + // thread, so forward the payload to `main` via std mpsc and block + // on `recv_timeout` below. `add_on_echo_fired_listener` is generated + // per `{.ffiEvent.}`-declared proc and takes a typed `&EchoEvent`. + let (tx, rx) = mpsc::channel::(); + let typed_handle = ctx.add_on_echo_fired_listener(move |evt: &EchoEvent| { + let _ = tx.send(evt.clone()); + }); + + // Wildcard listener: receives every event with the FFI return code, + // the wire `event_id` pre-extracted from the CBOR envelope, and the + // raw envelope bytes. Lift to a typed payload via + // `decode_event_payload::` when the event_id matches one you + // care about — this avoids hand-rolling ciborium calls per branch. + let wildcard_handle = ctx.add_event_listener(|ret: c_int, event_id: &str, envelope: &[u8]| { + println!("wildcard: ret={}, event_id={}, bytes={}", ret, event_id, envelope.len()); + if ret == 0 && event_id == "on_echo_fired" { + match decode_event_payload::(envelope) { + Ok(evt) => println!(" decoded: message={}, echo_count={}", evt.message, evt.echo_count), + Err(e) => println!(" decode failed: {}", e), + } + } + }); + + // Trigger the event — fires `on_echo_fired` once, which the + // dispatch thread delivers to both listeners above. + ctx.echo(EchoRequest { message: "sync-event-demo".into(), delay_ms: 1 })?; + + match rx.recv_timeout(Duration::from_secs(2)) { + Ok(evt) => println!("typed onEchoFired: message={}, echo_count={}", evt.message, evt.echo_count), + Err(e) => return Err(format!("event never arrived: {}", e)), + } + + ctx.remove_event_listener(typed_handle); + ctx.remove_event_listener(wildcard_handle); + Ok(()) +} diff --git a/examples/timer/rust_bindings/examples/tokio_main.rs b/examples/timer/rust_bindings/examples/tokio_main.rs new file mode 100644 index 0000000..8ab1d16 --- /dev/null +++ b/examples/timer/rust_bindings/examples/tokio_main.rs @@ -0,0 +1,60 @@ +//! Tokio (async) example: same shape as `main.rs` but exercises the +//! async `_async` API and bridges library events into a tokio-aware +//! channel for async consumption. +//! +//! Run with: `cargo run --example tokio_main` + +use my_timer::{decode_event_payload, EchoEvent, EchoRequest, MyTimerCtx, TimerConfig}; +use std::os::raw::c_int; +use std::time::Duration; +use tokio::sync::mpsc; + +#[tokio::main] +async fn main() -> Result<(), String> { + let ctx = MyTimerCtx::new_async( + TimerConfig { name: "rust-tokio-demo".into() }, + Duration::from_secs(5), + ) + .await?; + + // Typed listener: the handler fires on the lib's dispatch thread, + // which is *outside* the tokio runtime. Forwarding through a tokio + // `unbounded_channel` (Sender is Send + Sync, non-blocking) hands + // the event over to the runtime so we can `.await` it below. + let (typed_tx, mut typed_rx) = mpsc::unbounded_channel::(); + let typed_handle = ctx.add_on_echo_fired_listener(move |evt: &EchoEvent| { + let _ = typed_tx.send(evt.clone()); + }); + + // Wildcard listener: receives every event with the FFI return code, + // the wire `event_id` pre-extracted from the CBOR envelope, and the + // raw envelope bytes. Lift to a typed payload via + // `decode_event_payload::` when the event_id matches one you + // care about — this avoids hand-rolling ciborium calls per branch. + let wildcard_handle = ctx.add_event_listener(|ret: c_int, event_id: &str, envelope: &[u8]| { + println!("wildcard: ret={}, event_id={}, bytes={}", ret, event_id, envelope.len()); + if ret == 0 && event_id == "on_echo_fired" { + match decode_event_payload::(envelope) { + Ok(evt) => println!(" decoded: message={}, echo_count={}", evt.message, evt.echo_count), + Err(e) => println!(" decode failed: {}", e), + } + } + }); + + // Trigger an echo via the async API — fires `on_echo_fired` once, + // which the dispatch thread delivers to both listeners above. + ctx.echo_async(EchoRequest { message: "async-event-demo".into(), delay_ms: 1 }) + .await?; + + // Await the typed event with a bounded timeout so a missing event + // surfaces as an error instead of hanging the example forever. + let evt = tokio::time::timeout(Duration::from_secs(2), typed_rx.recv()) + .await + .map_err(|_| "event never arrived".to_string())? + .ok_or_else(|| "typed channel closed".to_string())?; + println!("typed onEchoFired: message={}, echo_count={}", evt.message, evt.echo_count); + + ctx.remove_event_listener(typed_handle); + ctx.remove_event_listener(wildcard_handle); + Ok(()) +} diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 4afbefb..709cfe4 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -74,6 +74,8 @@ proc generateCargoToml*(libName: string): string = # pulling its async-std/futures shims. # `tokio` is needed only for `tokio::time::timeout` around the async # `recv_async`. Feature-gating tokio (item 11) is a follow-up commit. + # `[dev-dependencies]` lets the bundled `examples/` use `#[tokio::main]` + # without pulling those features into the library's runtime profile. return """[package] name = "$1" @@ -85,6 +87,9 @@ serde = { version = "1", features = ["derive"] } ciborium = "0.2" flume = { version = "0.11", default-features = false, features = ["async"] } tokio = { version = "1", features = ["sync", "time"] } + +[dev-dependencies] +tokio = { version = "1", features = ["rt-multi-thread", "macros", "sync", "time"] } """ % [libName] From f96a5b158add9c321e33ff804f2275a5314a501b Mon Sep 17 00:00:00 2001 From: Ivan FB <128452529+Ivansete-status@users.noreply.github.com> Date: Tue, 2 Jun 2026 22:22:35 +0200 Subject: [PATCH 4/4] Remove wildcard event listener; keep per-event dispatch (#70) --- examples/echo/cpp_bindings/CMakeLists.txt | 6 +- examples/echo/cpp_bindings/echo.hpp | 4 +- examples/timer/cpp_bindings/CMakeLists.txt | 6 +- examples/timer/cpp_bindings/main.cpp | 41 +----- examples/timer/cpp_bindings/my_timer.hpp | 56 +------- examples/timer/rust_bindings/src/api.rs | 52 ------- ffi/codegen/cpp.nim | 130 ++---------------- ffi/codegen/rust.nim | 113 +-------------- ffi/codegen/templates/cpp/CMakeLists.txt.tpl | 6 +- .../templates/cpp/header_prelude.hpp.tpl | 4 +- ffi/ffi_context.nim | 3 +- ffi/ffi_events.nim | 90 +++++------- ffi/internal/ffi_library.nim | 5 +- tests/e2e/cpp/test_timer_e2e.cpp | 51 ------- tests/unit/test_event_dispatch.nim | 19 ++- tests/unit/test_event_listener.nim | 28 ++-- 16 files changed, 96 insertions(+), 518 deletions(-) diff --git a/examples/echo/cpp_bindings/CMakeLists.txt b/examples/echo/cpp_bindings/CMakeLists.txt index 6e1e25a..77dd7aa 100644 --- a/examples/echo/cpp_bindings/CMakeLists.txt +++ b/examples/echo/cpp_bindings/CMakeLists.txt @@ -1,10 +1,8 @@ cmake_minimum_required(VERSION 3.14) project(echo_cpp_bindings CXX C) -# The generated bindings target C++20. The event-listener API uses -# std::span on its wildcard callback to hand the -# CBOR envelope to consumers as a zero-copy view; only became -# part of the standard library in C++20. +# The generated bindings target C++20: designated initializers and other +# C++20 constructs are used throughout the emitted code. set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/examples/echo/cpp_bindings/echo.hpp b/examples/echo/cpp_bindings/echo.hpp index c6f50d0..efcfa2e 100644 --- a/examples/echo/cpp_bindings/echo.hpp +++ b/examples/echo/cpp_bindings/echo.hpp @@ -1,6 +1,6 @@ #pragma once -// Generated bindings require C++20 — the event-listener API uses -// std::span for the wildcard callback. +// Generated bindings require C++20 (designated initializers and other +// C++20 constructs are used throughout the emitted code). // MSVC keeps __cplusplus at 199711L unless /Zc:__cplusplus is passed, // so consult _MSVC_LANG when present (it always reflects the active // /std:c++XX level). diff --git a/examples/timer/cpp_bindings/CMakeLists.txt b/examples/timer/cpp_bindings/CMakeLists.txt index 0ead156..162d844 100644 --- a/examples/timer/cpp_bindings/CMakeLists.txt +++ b/examples/timer/cpp_bindings/CMakeLists.txt @@ -1,10 +1,8 @@ cmake_minimum_required(VERSION 3.14) project(my_timer_cpp_bindings CXX C) -# The generated bindings target C++20. The event-listener API uses -# std::span on its wildcard callback to hand the -# CBOR envelope to consumers as a zero-copy view; only became -# part of the standard library in C++20. +# The generated bindings target C++20: designated initializers and other +# C++20 constructs are used throughout the emitted code. set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/examples/timer/cpp_bindings/main.cpp b/examples/timer/cpp_bindings/main.cpp index 3ab361d..e754a85 100644 --- a/examples/timer/cpp_bindings/main.cpp +++ b/examples/timer/cpp_bindings/main.cpp @@ -92,54 +92,25 @@ int main() { // Each `{.ffiEvent.}` declared on the Nim side gets a typed // registration method — `addOnEchoFiredListener(handler)` here. - // A second `addEventListener` overload registers a catch-all - // wildcard listener that receives every event as raw envelope - // bytes plus the FFI return code. Both fire from the lib's + // Subscribe to each event separately; handlers fire from the lib's // dispatch thread, so synchronise via std::promise / atomics. std::promise echoEvtPromise; auto echoEvtFuture = echoEvtPromise.get_future(); const auto typedHandle = ctx->addOnEchoFiredListener( [&](const EchoEvent& evt) { echoEvtPromise.set_value(evt); }); - std::atomic wildcardHits{0}; - // Wildcard listener receives every event with the wire `eventId` - // pre-extracted plus a span view over the raw CBOR envelope - // bytes (zero-copy; valid only for the duration of this call). - // Dispatch on `eventId` and use `decodeEventPayload` to lift - // the payload into a typed value without hand-parsing CBOR. - const auto wildcardHandle = ctx->addEventListener( - [&](int retCode, const std::string& eventId, - std::span envelope) { - wildcardHits.fetch_add(1); - std::cout << "[7] wildcard event: retCode=" << retCode - << ", eventId=" << eventId - << ", envelope bytes=" << envelope.size() << "\n"; - if (retCode != 0) return; - if (eventId == "on_echo_fired") { - EchoEvent decoded{}; - if (decodeEventPayload(envelope, decoded)) { - std::cout << " decoded EchoEvent: message=" - << decoded.message - << ", echoCount=" << decoded.echoCount << "\n"; - } - } - }); - ctx->echo(EchoRequest{"event-demo", 1}); const auto evt = echoEvtFuture.get(); std::cout << "[7] typed event onEchoFired: message=" << evt.message - << ", echoCount=" << evt.echoCount - << ", wildcardHits=" << wildcardHits.load() << "\n"; + << ", echoCount=" << evt.echoCount << "\n"; - // Drop the typed listener — only the wildcard fires for the - // follow-up echo. Sleep briefly to give the lib thread time to - // deliver before we tear the ctx down. + // Drop the typed listener — no handler fires for the follow-up echo. + // Sleep briefly to give the lib thread time to settle before we tear + // the ctx down. ctx->removeEventListener(typedHandle); ctx->echo(EchoRequest{"event-demo-after-remove", 1}); std::this_thread::sleep_for(std::chrono::milliseconds(50)); - std::cout << "[7] after removeEventListener: wildcardHits=" - << wildcardHits.load() << "\n"; - ctx->removeEventListener(wildcardHandle); + std::cout << "[7] after removeEventListener: typed listener removed\n"; std::cout << "\nDone.\n"; return 0; diff --git a/examples/timer/cpp_bindings/my_timer.hpp b/examples/timer/cpp_bindings/my_timer.hpp index 8d1fc3d..8fa440b 100644 --- a/examples/timer/cpp_bindings/my_timer.hpp +++ b/examples/timer/cpp_bindings/my_timer.hpp @@ -1,6 +1,6 @@ #pragma once -// Generated bindings require C++20 — the event-listener API uses -// std::span for the wildcard callback. +// Generated bindings require C++20 (designated initializers and other +// C++20 constructs are used throughout the emitted code). // MSVC keeps __cplusplus at 199711L unless /Zc:__cplusplus is passed, // so consult _MSVC_LANG when present (it always reflects the active // /std:c++XX level). @@ -30,7 +30,6 @@ extern "C" { } #include -#include // ============================================================ // Result — exception-free error channel // ============================================================ @@ -773,19 +772,6 @@ inline Result> ffi_call_( #endif // NIM_FFI_SYNC_CALL_HELPER_HPP_INCLUDED -template -inline bool decodeEventPayload(std::span envelope, T& out) { - if (envelope.empty()) return false; - CborParser parser; CborValue it; - if (cbor_parser_init(envelope.data(), envelope.size(), 0, &parser, &it) != CborNoError) - return false; - if (!cbor_value_is_map(&it)) return false; - CborValue payloadField; - if (cbor_value_map_find_value(&it, "payload", &payloadField) != CborNoError) - return false; - return decode_cbor(payloadField, out) == CborNoError; -} - // ============================================================ // High-level C++ context class // ============================================================ @@ -853,16 +839,6 @@ public: return ListenerHandle{id}; } - ListenerHandle addEventListener(std::function)> handler) { - auto owned = std::make_unique(std::move(handler)); - auto* raw = owned.get(); - const auto id = my_timer_add_event_listener( - ptr_, "", &MyTimerCtx::wildcardTrampoline, raw); - if (id == 0) return ListenerHandle{0}; - listeners_.emplace(id, std::move(owned)); - return ListenerHandle{id}; - } - bool removeEventListener(ListenerHandle handle) { if (handle.id == 0) return false; const auto rc = my_timer_remove_event_listener(ptr_, handle.id); @@ -945,11 +921,6 @@ private: explicit TypedListener(std::function f) : fn(std::move(f)) {} }; - struct WildcardListener : ListenerBase { - std::function)> fn; - explicit WildcardListener(std::function)> f) : fn(std::move(f)) {} - }; - template static void typedTrampoline(int ret, const char* msg, std::size_t len, void* ud) { if (!ud || ret != 0 || !msg || len == 0) return; @@ -965,29 +936,6 @@ private: listener->fn(payload); } - static void wildcardTrampoline(int ret, const char* msg, std::size_t len, void* ud) { - if (!ud) return; - auto* listener = static_cast(ud); - if (!listener->fn) return; - std::span envelope{}; - if (msg && len > 0) { - envelope = std::span(reinterpret_cast(msg), len); - } - std::string eventId; - if (ret == 0 && !envelope.empty()) { - CborParser parser; CborValue it; - if (cbor_parser_init(envelope.data(), envelope.size(), 0, &parser, &it) == CborNoError - && cbor_value_is_map(&it)) { - CborValue evtField; - if (cbor_value_map_find_value(&it, "eventType", &evtField) == CborNoError - && cbor_value_is_text_string(&evtField)) { - (void)decode_cbor(evtField, eventId); - } - } - } - listener->fn(ret, eventId, envelope); - } - void* ptr_; std::chrono::milliseconds timeout_; std::unordered_map> listeners_; diff --git a/examples/timer/rust_bindings/src/api.rs b/examples/timer/rust_bindings/src/api.rs index d8d9721..709f9c1 100644 --- a/examples/timer/rust_bindings/src/api.rs +++ b/examples/timer/rust_bindings/src/api.rs @@ -117,48 +117,9 @@ unsafe extern "C" fn on_echo_fired_trampoline( } } -struct WildcardHandler { - f: Box, -} - -unsafe extern "C" fn my_timer_wildcard_trampoline( - ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void, -) { - if ud.is_null() { return; } - let h = &*(ud as *const WildcardHandler); - let bytes = if !msg.is_null() && len > 0 { - slice::from_raw_parts(msg as *const u8, len) - } else { &[] }; - let event_id = if ret == 0 && !bytes.is_empty() { - #[derive(serde::Deserialize)] - struct EnvelopeMeta { - #[serde(rename = "eventType")] - event_type: String, - } - ciborium::de::from_reader::(bytes) - .map(|m| m.event_type).unwrap_or_default() - } else { - String::new() - }; - (h.f)(ret, event_id.as_str(), bytes); -} - #[derive(Debug, Clone, Copy)] pub struct ListenerHandle { pub id: u64 } -/// Decode the `payload` field of a CBOR `EventEnvelope` as `T`. -/// Returns `Err` if the envelope is empty / malformed / the payload -/// cannot be deserialised as `T`. -pub fn decode_event_payload( - envelope: &[u8], -) -> Result { - #[derive(serde::Deserialize)] - struct Envelope { payload: T } - let env: Envelope = ciborium::de::from_reader(envelope) - .map_err(|e| format!("decode event payload: {e}"))?; - Ok(env.payload) -} - /// High-level context for `MyTimer`. pub struct MyTimerCtx { ptr: *mut c_void, @@ -237,19 +198,6 @@ impl MyTimerCtx { self.add_listener_inner(b"on_echo_fired\0".as_ptr() as *const c_char, on_echo_fired_trampoline, raw, owned) } - /// Register a catch-all listener that receives every event. - /// The handler arguments are (return_code, event_id, envelope_bytes): - /// `event_id` is the wire `eventType` string extracted from the - /// envelope (empty on error or malformed envelope); `envelope_bytes` - /// is the full CBOR envelope, suitable for `decode_event_payload::`. - pub fn add_event_listener(&self, handler: F) -> ListenerHandle - where F: Fn(c_int, &str, &[u8]) + Send + Sync + 'static, - { - let owned: Box = Box::new(WildcardHandler { f: Box::new(handler) }); - let raw = &*owned as *const WildcardHandler as *mut c_void; - self.add_listener_inner(b"\0".as_ptr() as *const c_char, my_timer_wildcard_trampoline, raw, owned) - } - /// Remove a previously-registered listener by handle. Returns true /// if the listener existed and was removed; false otherwise. pub fn remove_event_listener(&self, handle: ListenerHandle) -> bool { diff --git a/ffi/codegen/cpp.nim b/ffi/codegen/cpp.nim index 467b26b..3401a83 100644 --- a/ffi/codegen/cpp.nim +++ b/ffi/codegen/cpp.nim @@ -145,10 +145,7 @@ proc emitEventDispatcher( ## per declared `{.ffiEvent.}`. Internally registers under the wire ## event name; the per-listener trampoline decodes the CBOR ## envelope's `payload` field as `T` and invokes the user handler. - ## - `addEventListener(std::function) -> - ## ListenerHandle` registers a catch-all wildcard listener - ## (event_name == "") that receives every event as raw envelope - ## bytes plus the FFI return code. + ## Callers subscribe to each event separately. ## - `removeEventListener(ListenerHandle) -> bool` drops a listener by ## handle. After it returns true, no further callbacks for that id ## are in flight on the FFI side (the Nim-side registry lock plus @@ -189,30 +186,6 @@ proc emitEventDispatcher( lines.add(" return ListenerHandle{id};") lines.add(" }") lines.add("") - # Generic wildcard registration. - # - # The handler receives the FFI return code, the wire `eventType` string - # extracted from the CBOR envelope (empty if the envelope is malformed - # or `ret != 0`), and a `std::span` view over the raw envelope bytes — - # the buffer is owned by the dylib and stays valid for the duration of - # the synchronous callback only. Pair with `decodeEventPayload` to - # lift the payload into a typed value without hand-rolling CBOR parsing. - lines.add( - " ListenerHandle addEventListener(std::function)> handler) {" - ) - lines.add( - " auto owned = std::make_unique(std::move(handler));" - ) - lines.add(" auto* raw = owned.get();") - lines.add(" const auto id = $1_add_event_listener(" % [libName]) - lines.add( - " ptr_, \"\", &$1::wildcardTrampoline, raw);" % [ctxTypeName] - ) - lines.add(" if (id == 0) return ListenerHandle{0};") - lines.add(" listeners_.emplace(id, std::move(owned));") - lines.add(" return ListenerHandle{id};") - lines.add(" }") - lines.add("") # Remove by handle. lines.add(" bool removeEventListener(ListenerHandle handle) {") lines.add(" if (handle.id == 0) return false;") @@ -231,16 +204,10 @@ proc emitEventTrampoline( ## `emitEventDispatcher`: ## ## - `ListenerBase` is a polymorphic base so the context's - ## `listeners_` map can own typed and wildcard listeners under a - ## single value type. + ## `listeners_` map can own typed listeners under a single value type. ## - `TypedListener` holds the user's `std::function` ## and is the target of `typedTrampoline`, which CBOR-decodes the ## envelope's `payload` field as `T` and invokes the handler. - ## - `WildcardListener` holds a `std::function)>` and is the target - ## of `wildcardTrampoline`, which forwards the FFI return code plus - ## a span view over the raw payload bytes (no copy — the dylib owns - ## the buffer for the duration of the synchronous callback). if events.len == 0: return lines.add(" struct ListenerBase {") @@ -253,15 +220,6 @@ proc emitEventTrampoline( lines.add(" explicit TypedListener(std::function f) : fn(std::move(f)) {}") lines.add(" };") lines.add("") - lines.add(" struct WildcardListener : ListenerBase {") - lines.add( - " std::function)> fn;" - ) - lines.add( - " explicit WildcardListener(std::function)> f) : fn(std::move(f)) {}" - ) - lines.add(" };") - lines.add("") # Typed trampoline — one instantiation per payload type, all sharing a body. lines.add(" template ") lines.add(" static void typedTrampoline(int ret, const char* msg, std::size_t len, void* ud) {") @@ -284,45 +242,6 @@ proc emitEventTrampoline( lines.add(" listener->fn(payload);") lines.add(" }") lines.add("") - # Wildcard trampoline — extracts `eventType` from the CBOR envelope so - # the user can match on the event name without hand-parsing. Falls back - # to an empty `eventId` if the envelope is missing, malformed, or the - # FFI return code signals an error (in which case the bytes are an - # error string, not a CBOR envelope). - lines.add( - " static void wildcardTrampoline(int ret, const char* msg, std::size_t len, void* ud) {" - ) - lines.add(" if (!ud) return;") - lines.add(" auto* listener = static_cast(ud);") - lines.add(" if (!listener->fn) return;") - # The buffer pointed to by `msg` is owned by the dylib and stays valid - # for the duration of this synchronous call — safe to hand to the user - # as a span view rather than copying. - lines.add(" std::span envelope{};") - lines.add(" if (msg && len > 0) {") - lines.add( - " envelope = std::span(reinterpret_cast(msg), len);" - ) - lines.add(" }") - lines.add(" std::string eventId;") - lines.add(" if (ret == 0 && !envelope.empty()) {") - lines.add(" CborParser parser; CborValue it;") - lines.add( - " if (cbor_parser_init(envelope.data(), envelope.size(), 0, &parser, &it) == CborNoError" - ) - lines.add(" && cbor_value_is_map(&it)) {") - lines.add(" CborValue evtField;") - lines.add( - " if (cbor_value_map_find_value(&it, \"eventType\", &evtField) == CborNoError" - ) - lines.add(" && cbor_value_is_text_string(&evtField)) {") - lines.add(" (void)decode_cbor(evtField, eventId);") - lines.add(" }") - lines.add(" }") - lines.add(" }") - lines.add(" listener->fn(ret, eventId, envelope);") - lines.add(" }") - lines.add("") proc generateCppHeader*( procs: seq[FFIProcMeta], @@ -335,10 +254,8 @@ proc generateCppHeader*( lines.add(HeaderPreludeTpl) if events.len > 0: # Only pulled in when the library declares `{.ffiEvent.}` procs — - # `` backs the `listeners_` map, `` is the - # zero-copy view type handed to wildcard callbacks. + # `` backs the `listeners_` map. lines.add("#include ") - lines.add("#include ") # Result is the exception-free return channel used by every generated # entry point. It must precede the CBOR helpers and sync-call helper below, @@ -437,33 +354,6 @@ proc generateCppHeader*( lines.add(SyncCallHelperTpl) - # ── Event-payload decoder helper ────────────────────────────────────────── - # Lets wildcard-listener bodies lift the `payload` field out of a CBOR - # envelope into any registered event type with a single call, e.g. - # EchoEvent evt; - # if (decodeEventPayload(envelope, evt)) { ... } - # Relies on the per-struct `decode_cbor` codec emitted above. - if events.len > 0: - lines.add("template ") - lines.add( - "inline bool decodeEventPayload(std::span envelope, T& out) {" - ) - lines.add(" if (envelope.empty()) return false;") - lines.add(" CborParser parser; CborValue it;") - lines.add( - " if (cbor_parser_init(envelope.data(), envelope.size(), 0, &parser, &it) != CborNoError)" - ) - lines.add(" return false;") - lines.add(" if (!cbor_value_is_map(&it)) return false;") - lines.add(" CborValue payloadField;") - lines.add( - " if (cbor_value_map_find_value(&it, \"payload\", &payloadField) != CborNoError)" - ) - lines.add(" return false;") - lines.add(" return decode_cbor(payloadField, out) == CborNoError;") - lines.add("}") - lines.add("") - # ── High-level C++ context class ────────────────────────────────────────── var ctors: seq[FFIProcMeta] = @[] var methods: seq[FFIProcMeta] = @[] @@ -661,13 +551,13 @@ proc generateCppHeader*( lines.add("") lines.add("private:") - # Listener machinery (`ListenerBase`, `TypedListener`, - # `WildcardListener`, plus the static trampolines) must appear before - # the `listeners_` data member declaration — C++ requires the value - # type of a member to be complete at point of declaration. The public - # add*/remove methods above also reference these types, but member - # function bodies see the full class scope regardless of declaration - # order, so emitting here is sufficient for both. + # Listener machinery (`ListenerBase`, `TypedListener`, plus the + # static trampolines) must appear before the `listeners_` data member + # declaration — C++ requires the value type of a member to be complete + # at point of declaration. The public add*/remove methods above also + # reference these types, but member function bodies see the full class + # scope regardless of declaration order, so emitting here is sufficient + # for both. emitEventTrampoline(lines, events) lines.add(" void* ptr_;") lines.add(" std::chrono::milliseconds timeout_;") diff --git a/ffi/codegen/rust.nim b/ffi/codegen/rust.nim index 709cfe4..6972954 100644 --- a/ffi/codegen/rust.nim +++ b/ffi/codegen/rust.nim @@ -454,8 +454,8 @@ proc generateApiRs*( # ── Per-listener handler boxes + extern "C" trampolines ───────────────── # Each registered listener owns a `Box<…Handler>` that is kept alive in # `$1::listeners` (keyed by listener id). The raw pointer to the inner - # handler is handed to the dylib as `user_data` for the per-event or - # wildcard trampoline below. + # handler is handed to the dylib as `user_data` for the per-event + # trampoline below. if events.len > 0: for ev in events: let handlerStruct = capitalizeFirstLetter(ev.nimProcName) & "Handler" @@ -486,73 +486,11 @@ proc generateApiRs*( lines.add("}") lines.add("") - # Wildcard handler — receives every event as raw envelope bytes, - # the FFI return code, and the `eventType` string pre-extracted - # from the CBOR envelope. `event_id` is empty when `ret != 0` or - # the envelope is malformed (the bytes are an error string, not a - # CBOR envelope, in that case). - lines.add("struct WildcardHandler {") - lines.add(" f: Box,") - lines.add("}") - lines.add("") - lines.add("unsafe extern \"C\" fn $1_wildcard_trampoline(" % [libName]) - lines.add(" ret: c_int, msg: *const c_char, len: usize, ud: *mut c_void,") - lines.add(") {") - lines.add(" if ud.is_null() { return; }") - lines.add(" let h = &*(ud as *const WildcardHandler);") - lines.add(" let bytes = if !msg.is_null() && len > 0 {") - lines.add(" slice::from_raw_parts(msg as *const u8, len)") - lines.add(" } else { &[] };") - lines.add(" let event_id = if ret == 0 && !bytes.is_empty() {") - lines.add(" #[derive(serde::Deserialize)]") - lines.add(" struct EnvelopeMeta {") - lines.add(" #[serde(rename = \"eventType\")]") - lines.add(" event_type: String,") - lines.add(" }") - lines.add( - " ciborium::de::from_reader::(bytes)" - ) - lines.add(" .map(|m| m.event_type).unwrap_or_default()") - lines.add(" } else {") - lines.add(" String::new()") - lines.add(" };") - lines.add(" (h.f)(ret, event_id.as_str(), bytes);") - lines.add("}") - lines.add("") - # Public handle returned by every add_…_listener call. lines.add("#[derive(Debug, Clone, Copy)]") lines.add("pub struct ListenerHandle { pub id: u64 }") lines.add("") - # Helper: decode an event envelope's `payload` field into any typed - # `T` that the generated `types.rs` already derives `Deserialize` on. - # Pair with `add_event_listener` to lift raw envelope bytes into a - # typed payload without hand-rolling ciborium calls in each branch. - lines.add( - "/// Decode the `payload` field of a CBOR `EventEnvelope` as `T`." - ) - lines.add( - "/// Returns `Err` if the envelope is empty / malformed / the payload" - ) - lines.add("/// cannot be deserialised as `T`.") - lines.add( - "pub fn decode_event_payload(" - ) - lines.add(" envelope: &[u8],") - lines.add(") -> Result {") - lines.add(" #[derive(serde::Deserialize)]") - lines.add(" struct Envelope { payload: T }") - lines.add( - " let env: Envelope = ciborium::de::from_reader(envelope)" - ) - lines.add( - " .map_err(|e| format!(\"decode event payload: {e}\"))?;" - ) - lines.add(" Ok(env.payload)") - lines.add("}") - lines.add("") - # ── Context struct ───────────────────────────────────────────────────────── lines.add("/// High-level context for `$1`." % [libTypeName]) lines.add("pub struct $1 {" % [ctxTypeName]) @@ -697,11 +635,11 @@ proc generateApiRs*( # ── Listener-registration API ───────────────────────────────────────── if events.len > 0: # Private helper shared by every public `add_*_listener`: the - # FFI call + map insertion is identical across the typed and - # wildcard variants, so it lives in one place. The caller owns - # the box (typed as the concrete handler struct so the raw - # pointer matches the trampoline's expected type) and only - # erases it to `dyn Any + Send` when handing ownership over. + # FFI call + map insertion is identical across the typed event + # variants, so it lives in one place. The caller owns the box + # (typed as the concrete handler struct so the raw pointer matches + # the trampoline's expected type) and only erases it to + # `dyn Any + Send` when handing ownership over. lines.add(" fn add_listener_inner(") lines.add(" &self,") lines.add(" event_name: *const c_char,") @@ -751,43 +689,6 @@ proc generateApiRs*( lines.add(" }") lines.add("") - # Generic wildcard listener — receives every event with the wire - # `eventType` string pre-extracted plus the raw envelope bytes. Pair - # with `decode_event_payload::` to lift the payload into a typed - # value. - lines.add( - " /// Register a catch-all listener that receives every event." - ) - lines.add( - " /// The handler arguments are (return_code, event_id, envelope_bytes):" - ) - lines.add( - " /// `event_id` is the wire `eventType` string extracted from the" - ) - lines.add( - " /// envelope (empty on error or malformed envelope); `envelope_bytes`" - ) - lines.add( - " /// is the full CBOR envelope, suitable for `decode_event_payload::`." - ) - lines.add( - " pub fn add_event_listener(&self, handler: F) -> ListenerHandle" - ) - lines.add(" where F: Fn(c_int, &str, &[u8]) + Send + Sync + 'static,") - lines.add(" {") - lines.add( - " let owned: Box = Box::new(WildcardHandler { f: Box::new(handler) });" - ) - lines.add( - " let raw = &*owned as *const WildcardHandler as *mut c_void;" - ) - lines.add( - " self.add_listener_inner(b\"\\0\".as_ptr() as *const c_char, $1_wildcard_trampoline, raw, owned)" % - [libName] - ) - lines.add(" }") - lines.add("") - # Remove by handle. Drops the Box (and the user's closure) after the # C ABI confirms the listener has been unregistered. lines.add( diff --git a/ffi/codegen/templates/cpp/CMakeLists.txt.tpl b/ffi/codegen/templates/cpp/CMakeLists.txt.tpl index 9310115..b204d4e 100644 --- a/ffi/codegen/templates/cpp/CMakeLists.txt.tpl +++ b/ffi/codegen/templates/cpp/CMakeLists.txt.tpl @@ -1,10 +1,8 @@ cmake_minimum_required(VERSION 3.14) project({{LIB}}_cpp_bindings CXX C) -# The generated bindings target C++20. The event-listener API uses -# std::span on its wildcard callback to hand the -# CBOR envelope to consumers as a zero-copy view; only became -# part of the standard library in C++20. +# The generated bindings target C++20: designated initializers and other +# C++20 constructs are used throughout the emitted code. set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl index 6180dd4..66029b6 100644 --- a/ffi/codegen/templates/cpp/header_prelude.hpp.tpl +++ b/ffi/codegen/templates/cpp/header_prelude.hpp.tpl @@ -1,6 +1,6 @@ #pragma once -// Generated bindings require C++20 — the event-listener API uses -// std::span for the wildcard callback. +// Generated bindings require C++20 (designated initializers and other +// C++20 constructs are used throughout the emitted code). // MSVC keeps __cplusplus at 199711L unless /Zc:__cplusplus is passed, // so consult _MSVC_LANG when present (it always reflects the active // /std:c++XX level). diff --git a/ffi/ffi_context.nim b/ffi/ffi_context.nim index 7dcb01b..531347c 100644 --- a/ffi/ffi_context.nim +++ b/ffi/ffi_context.nim @@ -107,8 +107,7 @@ proc onNotResponding*(ctx: ptr FFIContext) = ## Mirrors the dispatch templates' lock-during-invocation contract ## (see `ffi_events.nim`). withLock ctx[].eventRegistry.lock: - let snap = ctx[].eventRegistry.byEvent.getOrDefault("onNotResponding") & - ctx[].eventRegistry.wildcard + let snap = ctx[].eventRegistry.byEvent.getOrDefault("onNotResponding") if snap.len == 0: chronicles.debug "onNotResponding - no listener registered" return diff --git a/ffi/ffi_events.nim b/ffi/ffi_events.nim index 09cfa0f..0a33308 100644 --- a/ffi/ffi_events.nim +++ b/ffi/ffi_events.nim @@ -3,9 +3,9 @@ ## This module owns two concerns so they can evolve together without dragging ## in the rest of `FFIContext`: ## -## 1. A multi-listener registry. Each event name maps to a `seq` of listeners; -## the empty event name `""` is the wildcard channel and receives every -## dispatched event in addition to its own per-name subscribers. +## 1. A multi-listener registry. Each event name maps to a `seq` of +## listeners; a dispatched event reaches exactly the listeners +## subscribed to its name. Callers subscribe to each event separately. ## 2. The dispatch templates (`dispatchFFIEvent`, `dispatchFFIEventCbor`) used ## by `{.ffiEvent.}`-generated procs. They snapshot the registry under its ## lock, then invoke each listener *outside* the lock so re-entrant @@ -17,7 +17,7 @@ {.pragma: callback, cdecl, raises: [], gcsafe.} -import std/[locks, tables] +import std/[locks, sequtils, tables] import chronicles import ./ffi_types, ./cbor_serial @@ -49,10 +49,6 @@ type lock*: Lock nextId*: uint64 ## Monotonic id source. 0 is reserved as "invalid"; ids start at 1. byEvent*: Table[string, seq[FFIEventListener]] - wildcard*: seq[FFIEventListener] - -const WildcardEventName* = "" - ## Empty string registers a wildcard listener that receives every event. # --------------------------------------------------------------------------- # Registry lifecycle and mutation @@ -66,7 +62,6 @@ proc initEventRegistry*(reg: var FFIEventRegistry) = reg.lock.initLock() reg.nextId = 0'u64 reg.byEvent = initTable[string, seq[FFIEventListener]]() - reg.wildcard.setLen(0) proc deinitEventRegistry*(reg: var FFIEventRegistry) = ## Mirror of `initEventRegistry`: must be called exactly once, by the @@ -80,7 +75,6 @@ proc deinitEventRegistry*(reg: var FFIEventRegistry) = ## assignment destructor against this thread's heap allocations. reg.lock.deinitLock() reg.byEvent = default(Table[string, seq[FFIEventListener]]) - reg.wildcard = @[] reg.nextId = 0'u64 proc addEventListener*( @@ -90,9 +84,10 @@ proc addEventListener*( userData: pointer, ): uint64 {.raises: [].} = ## Registers `callback` for `eventName` and returns the listener's stable - ## id (always non-zero on success). `eventName == ""` registers a wildcard - ## listener that receives every dispatched event. Returns 0 if `callback` - ## is nil — the only documented failure mode. + ## id (always non-zero on success). A listener only receives events + ## dispatched under its own `eventName` — subscribe to each event + ## separately. Returns 0 if `callback` is nil — the only documented + ## failure mode. if callback.isNil(): return 0 @@ -103,10 +98,7 @@ proc addEventListener*( assigned = reg.nextId let listener = FFIEventListener(id: assigned, callback: callback, userData: userData) - if eventName.len == 0: - reg.wildcard.add(listener) - else: - reg.byEvent.mgetOrPut(eventName, @[]).add(listener) + reg.byEvent.mgetOrPut(eventName, @[]).add(listener) return assigned proc removeEventListener*(reg: var FFIEventRegistry, id: uint64): bool {.raises: [].} = @@ -120,54 +112,41 @@ proc removeEventListener*(reg: var FFIEventRegistry, id: uint64): bool {.raises: var removed = false withLock reg.lock: - for i in 0 ..< reg.wildcard.len: - if reg.wildcard[i].id == id: - reg.wildcard.delete(i) + var + pruneKey = "" + prune = false + for key, listeners in reg.byEvent.mpairs: + let before = listeners.len + listeners.keepItIf(it.id != id) + if listeners.len < before: removed = true + if listeners.len == 0: + pruneKey = key + prune = true break - if not removed: - var emptyKey = "" - var prune = false - for key, listeners in reg.byEvent.mpairs: - var idx = -1 - for i in 0 ..< listeners.len: - if listeners[i].id == id: - idx = i - break - if idx >= 0: - listeners.delete(idx) - removed = true - if listeners.len == 0: - emptyKey = key - prune = true - break - if prune: - reg.byEvent.del(emptyKey) + if prune: + reg.byEvent.del(pruneKey) return removed proc removeAllEventListeners*(reg: var FFIEventRegistry) {.raises: [].} = - ## Drops every registered listener (per-event and wildcard). Does not - ## reset the listener-id counter — subsequent `addEventListener` calls - ## still return strictly increasing ids. + ## Drops every registered listener. Does not reset the listener-id + ## counter — subsequent `addEventListener` calls still return strictly + ## increasing ids. withLock reg.lock: - reg.wildcard.setLen(0) reg.byEvent.clear() proc snapshotListeners*( reg: var FFIEventRegistry, eventName: string ): seq[FFIEventListener] {.raises: [].} = - ## Returns a copy of the listener slice for `eventName`, plus every - ## wildcard listener. The copy is what makes re-entrant add/remove from - ## inside a handler deadlock-free: dispatch holds the lock only for the - ## duration of the copy, then iterates the copy outside the lock. + ## Returns a copy of the listener slice for `eventName`. The copy is what + ## makes re-entrant add/remove from inside a handler deadlock-free: + ## dispatch holds the lock only for the duration of the copy, then + ## iterates the copy outside the lock. var snap: seq[FFIEventListener] = @[] withLock reg.lock: - if eventName.len > 0: - # `getOrDefault` returns an empty seq when the key is absent — - # avoids the raising `[]` operator path. - for l in reg.byEvent.getOrDefault(eventName): - snap.add(l) - for l in reg.wildcard: + # `getOrDefault` returns an empty seq when the key is absent — + # avoids the raising `[]` operator path. + for l in reg.byEvent.getOrDefault(eventName): snap.add(l) return snap @@ -192,8 +171,7 @@ template withFFIEventDispatch( return withLock regPtr[].lock: - let listeners = - regPtr[].byEvent.getOrDefault(eventName) & regPtr[].wildcard + let listeners = regPtr[].byEvent.getOrDefault(eventName) if listeners.len == 0: chronicles.debug eventName & " - no listener registered" else: @@ -212,8 +190,8 @@ template withFFIEventDispatch( ) template dispatchFFIEvent*(eventName: string, body: untyped) = - ## Dispatches an FFI event to every listener for `eventName` plus every - ## wildcard listener. `body` must yield a `string` or `seq[byte]`. + ## Dispatches an FFI event to every listener subscribed to `eventName`. + ## `body` must yield a `string` or `seq[byte]`. ## ## Valid only on the FFI thread (where `ffiCurrentEventRegistry` is ## set). Holds `reg.lock` for the entire snapshot + invocation so a diff --git a/ffi/internal/ffi_library.nim b/ffi/internal/ffi_library.nim index 0fcd26c..824f54b 100644 --- a/ffi/internal/ffi_library.nim +++ b/ffi/internal/ffi_library.nim @@ -112,8 +112,9 @@ macro declareLibrary*(libraryName: static[string], libType: untyped): untyped = ## ABI on its `FFIContext`: ## ## - `{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). + ## — registers `cb` for `event_name` and returns its stable id. `cb` + ## only receives events dispatched under `event_name`; subscribe to + ## each event separately. ## - `{libraryName}_remove_event_listener(ctx, id) -> cint` — returns 0 on ## success, non-zero if no listener with that id exists. ## diff --git a/tests/e2e/cpp/test_timer_e2e.cpp b/tests/e2e/cpp/test_timer_e2e.cpp index a181d65..1f882a2 100644 --- a/tests/e2e/cpp/test_timer_e2e.cpp +++ b/tests/e2e/cpp/test_timer_e2e.cpp @@ -404,54 +404,3 @@ TEST(TimerE2E, RemoveEventListenerStopsDelivery) { EXPECT_EQ(keptHits.load(), 2); EXPECT_EQ(removedHits.load(), 1) << "removed listener fired after removeEventListener"; } - -// The wildcard `addEventListener` overload receives every event with the -// wire `eventId` pre-extracted plus a `std::span` view over the raw -// envelope bytes. The helper `decodeEventPayload` lifts the payload -// into a typed value. -TEST(TimerE2E, WildcardListenerReceivesEventIdAndDecodesPayload) { - auto ctx = makeCtx("wildcard"); - - struct Capture { - int retCode; - std::string eventId; - std::size_t envelopeBytes; - std::optional decoded; - }; - - std::mutex mu; - std::vector captured; - auto handle = ctx->addEventListener( - [&](int retCode, const std::string& eventId, - std::span envelope) { - Capture c{retCode, eventId, envelope.size(), std::nullopt}; - if (retCode == 0 && eventId == "on_echo_fired") { - EchoEvent evt{}; - if (decodeEventPayload(envelope, evt)) { - c.decoded = evt; - } - } - std::lock_guard lock(mu); - captured.push_back(std::move(c)); - }); - ASSERT_NE(handle.id, 0u); - - ctx->echo(EchoRequest{"hello", 1}); - - for (int i = 0; i < 200; ++i) { - { - std::lock_guard lock(mu); - if (!captured.empty()) break; - } - std::this_thread::sleep_for(std::chrono::milliseconds(5)); - } - - std::lock_guard lock(mu); - ASSERT_GE(captured.size(), 1u); - EXPECT_EQ(captured.front().retCode, 0); - EXPECT_EQ(captured.front().eventId, "on_echo_fired"); - EXPECT_GT(captured.front().envelopeBytes, 0u); - ASSERT_TRUE(captured.front().decoded.has_value()); - EXPECT_EQ(captured.front().decoded->message, "hello"); - EXPECT_EQ(captured.front().decoded->echoCount, 1); -} diff --git a/tests/unit/test_event_dispatch.nim b/tests/unit/test_event_dispatch.nim index 8db4ba4..c5bd198 100644 --- a/tests/unit/test_event_dispatch.nim +++ b/tests/unit/test_event_dispatch.nim @@ -86,8 +86,8 @@ registerReqFFI(EmitRawBytesEventRequest, lib: ptr TestEvtLib): return ok("emitted") ## Setter-thread worker for the registry race regression test. Each -## iteration adds then immediately removes a wildcard listener so a -## TSan-instrumented build can confirm `FFIEventRegistry.lock` +## iteration adds then immediately removes a listener for the dispatched +## event so a TSan-instrumented build can confirm `FFIEventRegistry.lock` ## serialises the cross-thread mutation against dispatch-time ## `snapshotListeners` reads from the FFI thread. type SetterArgs = tuple @@ -98,7 +98,7 @@ type SetterArgs = tuple proc setterThreadBody(args: SetterArgs) {.thread.} = while not args.stop[].load(): let id = addEventListener( - args.ctx[].eventRegistry, WildcardEventName, captureCb, args.target + args.ctx[].eventRegistry, "message_sent", captureCb, args.target ) discard removeEventListener(args.ctx[].eventRegistry, id) @@ -116,10 +116,9 @@ suite "dispatchFFIEventCbor": defer: deinitCallbackData(evt) - # Register the event callback via the same locked helper that the - # codegen-emitted `{libname}_set_event_callback` uses. + # Subscribe to the specific event the request below dispatches. discard addEventListener( - ctx[].eventRegistry, WildcardEventName, captureCb, addr evt + ctx[].eventRegistry, "message_sent", captureCb, addr evt ) # Trigger the dispatch from the FFI thread; the response callback is @@ -159,7 +158,7 @@ suite "dispatchFFIEvent with seq[byte]": deinitCallbackData(evt) discard addEventListener( - ctx[].eventRegistry, WildcardEventName, captureCb, addr evt + ctx[].eventRegistry, "raw_bytes", captureCb, addr evt ) var rsp: CallbackData @@ -178,8 +177,8 @@ suite "dispatchFFIEvent with seq[byte]": check callbackBytes(evt) == @[byte 0x01, 0x02, 0x03] when not defined(gcRefc): - ## Skipped under `--mm:refc`: each setter thread grows / shrinks - ## `reg.wildcard` (a `seq[FFIEventListener]`) via `addEventListener`, + ## Skipped under `--mm:refc`: each setter thread grows / shrinks the + ## per-event listener `seq[FFIEventListener]` via `addEventListener`, ## and refc's per-thread GC heap ownership makes cross-thread seq ## buffer reallocation unsafe even when the surrounding lock is held. ## ORC + the FFI thread + tsan (the combo this test was written for) @@ -206,7 +205,7 @@ when not defined(gcRefc): # (callback, userData) pair — what matters is the cross-thread write # racing the FFI thread's read, not which pair "wins". discard addEventListener( - ctx[].eventRegistry, WildcardEventName, captureCb, addr evt + ctx[].eventRegistry, "message_sent", captureCb, addr evt ) const NumSetterThreads = 4 diff --git a/tests/unit/test_event_listener.nim b/tests/unit/test_event_listener.nim index 432b6ba..620c385 100644 --- a/tests/unit/test_event_listener.nim +++ b/tests/unit/test_event_listener.nim @@ -68,7 +68,7 @@ suite "FFIEventRegistry mutation": let id1 = addEventListener(reg, "evt", tagCb, addr t) let id2 = addEventListener(reg, "evt", tagCb, addr t) - let id3 = addEventListener(reg, "", tagCb, addr t) + let id3 = addEventListener(reg, "other", tagCb, addr t) check id1 == 1'u64 check id2 == 2'u64 check id3 == 3'u64 @@ -89,7 +89,7 @@ suite "FFIEventRegistry mutation": check not removeEventListener(reg, 0'u64) check not removeEventListener(reg, 99'u64) - test "removeEventListener removes from per-event seq and wildcard": + test "removeEventListener removes listeners across distinct events": var reg: FFIEventRegistry initEventRegistry(reg) defer: @@ -101,18 +101,18 @@ suite "FFIEventRegistry mutation": var t = Tag(name: "a", rec: addr rec) let id1 = addEventListener(reg, "evt", tagCb, addr t) - let id2 = addEventListener(reg, "", tagCb, addr t) + let id2 = addEventListener(reg, "other", tagCb, addr t) check removeEventListener(reg, id1) check removeEventListener(reg, id2) # Second remove of the same id is a no-op. check not removeEventListener(reg, id1) - let snap = snapshotListeners(reg, "evt") - check snap.len == 0 + check snapshotListeners(reg, "evt").len == 0 + check snapshotListeners(reg, "other").len == 0 suite "FFIEventRegistry snapshot semantics": - test "snapshot includes both per-event listeners and wildcards": + test "snapshot returns only the listeners for the requested event": var reg: FFIEventRegistry initEventRegistry(reg) defer: @@ -126,17 +126,17 @@ suite "FFIEventRegistry snapshot semantics": var c = Tag(name: "c", rec: addr rec) discard addEventListener(reg, "evt", tagCb, addr a) - discard addEventListener(reg, "other", tagCb, addr b) - discard addEventListener(reg, "", tagCb, addr c) + discard addEventListener(reg, "evt", tagCb, addr b) + discard addEventListener(reg, "other", tagCb, addr c) let snapEvt = snapshotListeners(reg, "evt") - check snapEvt.len == 2 # listener for "evt" + wildcard + check snapEvt.len == 2 # both listeners for "evt" let snapOther = snapshotListeners(reg, "other") - check snapOther.len == 2 # listener for "other" + wildcard + check snapOther.len == 1 # only the listener for "other" let snapUnknown = snapshotListeners(reg, "no-subscriber") - check snapUnknown.len == 1 # only the wildcard + check snapUnknown.len == 0 # no listener for this event test "snapshot is a copy: post-snapshot mutation does not affect it": var reg: FFIEventRegistry @@ -161,7 +161,7 @@ suite "FFIEventRegistry snapshot semantics": check snap[0].id == id1 suite "removeAllEventListeners": - test "drops every listener (per-event and wildcard)": + test "drops every registered listener": var reg: FFIEventRegistry initEventRegistry(reg) defer: @@ -174,8 +174,8 @@ suite "removeAllEventListeners": var b = Tag(name: "b", rec: addr rec) discard addEventListener(reg, "evt", tagCb, addr a) - discard addEventListener(reg, WildcardEventName, tagCb, addr b) + discard addEventListener(reg, "other", tagCb, addr b) removeAllEventListeners(reg) check snapshotListeners(reg, "evt").len == 0 - check snapshotListeners(reg, WildcardEventName).len == 0 + check snapshotListeners(reg, "other").len == 0