From 549d369e6e9236d35bcdecca5bc3fdcae8edf481 Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 3 Jun 2026 09:54:44 -0600 Subject: [PATCH 1/4] Panic event + unit test --- AGENTS.md | 2 +- src/ffi_client.cpp | 14 ++++++++-- src/ffi_client.h | 5 ++-- src/tests/unit/test_ffi_client.cpp | 45 ++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e234ec3a..da247b49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ When making larger scale changes, check with the developer before committing to The SDK has three categories of threads: -**FFI callback thread** — The Rust FFI layer calls `LivekitFfiCallback` from a Rust-managed thread (typically a Tokio runtime thread). This single entry point deserializes the `FfiEvent` and calls `FfiClient::pushEvent`, which: +**FFI callback thread** — The Rust FFI layer calls `ffiEventCallback` from a Rust-managed thread (typically a Tokio runtime thread). This single entry point deserializes the `FfiEvent` and calls `FfiClient::pushEvent`, which: 1. Completes any pending async `std::promise` matched by `async_id`. 2. Invokes all registered `FfiClient` listeners (including `Room::onEvent`). diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 1ae3ddf4..3cd93b1c 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -17,6 +17,8 @@ #include "ffi_client.h" #include +#include +#include #include "data_track.pb.h" #include "e2ee.pb.h" @@ -167,7 +169,7 @@ bool FfiClient::initialize(bool capture_logs) { return false; } initialized_.store(true, std::memory_order_release); - livekit_ffi_initialize(&LivekitFfiCallback, capture_logs, LIVEKIT_BUILD_FLAVOR, LIVEKIT_BUILD_VERSION); + livekit_ffi_initialize(&ffiEventCallback, capture_logs, LIVEKIT_BUILD_FLAVOR, LIVEKIT_BUILD_VERSION); return true; } @@ -250,11 +252,19 @@ void FfiClient::pushEvent(const proto::FfiEvent& event) const { } } -void LivekitFfiCallback(const uint8_t* buf, size_t len) { +extern "C" LIVEKIT_INTERNAL_API void ffiEventCallback(const uint8_t* buf, size_t len) { proto::FfiEvent event; event.ParseFromArray(buf, static_cast(len)); // TODO: this fixes for now, what if len exceeds int? + // We are in a unrecoverable state, terminate the process + // This is what Python does, may not make sense for C++ + if (event.has_panic()) { + std::cerr << "FFI Panic: " << event.panic().message() << '\n'; + std::raise(SIGTERM); + return; + } + FfiClient::instance().pushEvent(event); } diff --git a/src/ffi_client.h b/src/ffi_client.h index 5136e49e..5ea0a89a 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -59,7 +58,7 @@ extern "C" void livekit_ffi_initialize(FfiCallbackFn cb, bool capture_logs, cons extern "C" void livekit_ffi_dispose(); -extern "C" void LivekitFfiCallback(const uint8_t* buf, size_t len); +extern "C" LIVEKIT_INTERNAL_API void ffiEventCallback(const uint8_t* buf, size_t len); // The FfiClient is used to communicate with the FFI interface of the Rust SDK // We use the generated protocol messages to facilitate the communication. @@ -195,7 +194,7 @@ class LIVEKIT_INTERNAL_API FfiClient { std::atomic next_async_id_{1}; void pushEvent(const proto::FfiEvent& event) const; - friend void LivekitFfiCallback(const uint8_t* buf, size_t len); + friend void ffiEventCallback(const uint8_t* buf, size_t len); std::atomic initialized_{false}; }; } // namespace livekit diff --git a/src/tests/unit/test_ffi_client.cpp b/src/tests/unit/test_ffi_client.cpp index d240654b..f6982ebb 100644 --- a/src/tests/unit/test_ffi_client.cpp +++ b/src/tests/unit/test_ffi_client.cpp @@ -17,7 +17,9 @@ #include #include +#include #include +#include #include #include "ffi.pb.h" @@ -25,6 +27,19 @@ namespace livekit::test { +namespace { + +volatile bool g_sigterm_received = false; + +// Has to be registered globally per csignal API +void handleSignal(int signal) { + if (signal == SIGTERM) { + g_sigterm_received = true; + } +} + +} // namespace + class FfiClientTest : public ::testing::Test { protected: void SetUp() override { @@ -140,6 +155,36 @@ TEST_F(FfiClientTest, ListenerRegistrationSurvivesShutdownReinitCycle) { EXPECT_NO_THROW(FfiClient::instance().removeListener(id)); } +TEST_F(FfiClientTest, PanicEvent) { + // Wire up a signal handler to ensure the panic event raises SIGTERM + // (and that users can handle it) + g_sigterm_received = false; + auto previous_handler = std::signal(SIGTERM, handleSignal); + ASSERT_NE(previous_handler, SIG_ERR); + + // Wire up a listener to ensure the panic event doesn't make it through + // (matches Python SDK) + bool listener_called = false; + const auto id = + FfiClient::instance().addListener([&listener_called](const proto::FfiEvent&) { listener_called = true; }); + + proto::FfiEvent event; + event.mutable_panic()->set_message("rust panic"); + std::string bytes; + ASSERT_TRUE(event.SerializeToString(&bytes)); + + testing::internal::CaptureStderr(); + ffiEventCallback(reinterpret_cast(bytes.data()), bytes.size()); + const std::string stderr_output = testing::internal::GetCapturedStderr(); + + ASSERT_NE(std::signal(SIGTERM, previous_handler), SIG_ERR); + FfiClient::instance().removeListener(id); + + EXPECT_TRUE(g_sigterm_received); + EXPECT_FALSE(listener_called); + EXPECT_NE(stderr_output.find("FFI Panic: rust panic"), std::string::npos); +} + // --------------------------------------------------------------------------- // These tests ensure FfiClient methods throw in various error conditions // --------------------------------------------------------------------------- From 251cca7bbc6fe6f2e18c7609ec3ccd1c8a1c1bd7 Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 3 Jun 2026 10:02:36 -0600 Subject: [PATCH 2/4] Remove iostream --- src/ffi_client.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 3cd93b1c..e4a07e9c 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -18,7 +18,6 @@ #include #include -#include #include "data_track.pb.h" #include "e2ee.pb.h" @@ -260,7 +259,7 @@ extern "C" LIVEKIT_INTERNAL_API void ffiEventCallback(const uint8_t* buf, size_t // We are in a unrecoverable state, terminate the process // This is what Python does, may not make sense for C++ if (event.has_panic()) { - std::cerr << "FFI Panic: " << event.panic().message() << '\n'; + LK_LOG_ERROR("FFI Panic: {}", event.panic().message()); std::raise(SIGTERM); return; } From 26782573a8cb953bf38357175913fb5a98a02aab Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 3 Jun 2026 11:11:46 -0600 Subject: [PATCH 3/4] Remove sketch comment --- src/ffi_client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index e4a07e9c..5262c2ad 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -257,7 +257,6 @@ extern "C" LIVEKIT_INTERNAL_API void ffiEventCallback(const uint8_t* buf, size_t static_cast(len)); // TODO: this fixes for now, what if len exceeds int? // We are in a unrecoverable state, terminate the process - // This is what Python does, may not make sense for C++ if (event.has_panic()) { LK_LOG_ERROR("FFI Panic: {}", event.panic().message()); std::raise(SIGTERM); From 4d058b75be07b3b96c02e79e9c9708d351cb708c Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 3 Jun 2026 15:19:50 -0600 Subject: [PATCH 4/4] Use critical log and flush --- src/ffi_client.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 5262c2ad..0966816f 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -258,7 +258,8 @@ extern "C" LIVEKIT_INTERNAL_API void ffiEventCallback(const uint8_t* buf, size_t // We are in a unrecoverable state, terminate the process if (event.has_panic()) { - LK_LOG_ERROR("FFI Panic: {}", event.panic().message()); + LK_LOG_CRITICAL("FFI Panic: {}", event.panic().message()); + livekit::detail::getLogger()->flush(); // Flush the logger to ensure all messages are written std::raise(SIGTERM); return; }