From 59bc2fd69f9bc175e509585b96ab37dca3072736 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 28 May 2026 22:22:16 -0600 Subject: [PATCH 1/6] Initial attempt at fixing room disconnect issue report --- include/livekit/room.h | 20 +++++ src/ffi_client.cpp | 31 ++++++++ src/ffi_client.h | 5 ++ src/room.cpp | 109 ++++++++++++++++++++++++++-- src/room_proto_converter.cpp | 77 +++++++++++++++++++- src/room_proto_converter.h | 1 + src/tests/integration/test_room.cpp | 62 ++++++++++++++++ 7 files changed, 296 insertions(+), 9 deletions(-) diff --git a/include/livekit/room.h b/include/livekit/room.h index ac80fb4d..b7e0ef1a 100644 --- a/include/livekit/room.h +++ b/include/livekit/room.h @@ -141,6 +141,26 @@ class LIVEKIT_API Room { bool Connect(const std::string& url, const std::string& token, const RoomOptions& options); // NOLINTEND(readability-identifier-naming) + /// Gracefully disconnect from the room. + /// + /// Sends a `DisconnectRequest` to the server with the given reason, blocks + /// until the FFI acknowledges, and tears down all room state (participants, + /// listener, E2EE manager, subscription threads). The `onDisconnected` + /// delegate is invoked once with the supplied reason. + /// + /// Safe to call from any thread, but **must not** be called from inside a + /// `RoomDelegate` callback — doing so will deadlock the event listener. + /// + /// @note `~Room()` invokes `disconnect()` automatically if the room is + /// still connected, so explicit calls are optional. Calling this + /// explicitly lets you handle the disconnect outcome and choose a + /// reason other than `ClientInitiated`. + /// + /// @param reason Reason reported to the server (default: ClientInitiated). + /// @returns true if a disconnect roundtrip was performed; false if the + /// room was already disconnected. + bool disconnect(DisconnectReason reason = DisconnectReason::ClientInitiated); + // Accessors /// Retrieve static metadata about the room. diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index de2ae065..1ae3ddf4 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -380,6 +380,37 @@ std::future FfiClient::connectAsync(const std::string& u return fut; } +std::future FfiClient::disconnectAsync(uintptr_t room_handle, DisconnectReason reason) { + const AsyncId async_id = generateAsyncId(); + + auto fut = registerAsync( + async_id, + // match: this DisconnectCallback's async_id + [async_id](const proto::FfiEvent& event) { + return event.has_disconnect() && event.disconnect().async_id() == async_id; + }, + // handler: nothing to extract; the callback is signal-only + [](const proto::FfiEvent& /*event*/, std::promise& pr) { pr.set_value(); }); + + proto::FfiRequest req; + auto* disconnect = req.mutable_disconnect(); + disconnect->set_room_handle(room_handle); + disconnect->set_request_async_id(async_id); + disconnect->set_reason(toProto(reason)); + + try { + const proto::FfiResponse resp = sendRequest(req); + if (!resp.has_disconnect()) { + logAndThrow("FfiResponse missing disconnect"); + } + } catch (...) { + cancelPendingByAsyncId(async_id); + throw; + } + + return fut; +} + // Track APIs Implementation std::future> FfiClient::getTrackStatsAsync(uintptr_t track_handle) { // Generate client-side async_id first diff --git a/src/ffi_client.h b/src/ffi_client.h index 1cfdd361..f595cb98 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -30,6 +30,7 @@ #include "data_track.pb.h" #include "livekit/data_track_error.h" #include "livekit/result.h" +#include "livekit/room_event_types.h" #include "livekit/stats.h" #include "livekit/visibility.h" #include "lk_log.h" @@ -94,6 +95,10 @@ class LIVEKIT_INTERNAL_API FfiClient { std::future connectAsync(const std::string& url, const std::string& token, const RoomOptions& options); + // Send a DisconnectRequest for the given room handle and wait for the + // FFI's DisconnectCallback. Throws std::runtime_error on FFI failure. + std::future disconnectAsync(uintptr_t room_handle, DisconnectReason reason); + // Track APIs std::future> getTrackStatsAsync(uintptr_t track_handle); diff --git a/src/room.cpp b/src/room.cpp index 99b050a6..dde2b46d 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -76,6 +76,21 @@ void readyForRoomEvent(std::uint64_t room_handle) { Room::Room() : subscription_thread_dispatcher_(std::make_unique()) {} Room::~Room() { + // Issue a graceful disconnect so the server sees us leave instead of + // timing out (RAII expectation; see issue #118). disconnect() does the + // full teardown including subscription threads, listener, and local + // participant, so the destructor only needs to handle the + // already-disconnected path. + try { + disconnect(); + } catch (const std::exception& e) { + LK_LOG_ERROR("Room::~Room: graceful disconnect failed: {}", e.what()); + } catch (...) { + LK_LOG_ERROR("Room::~Room: graceful disconnect failed: unknown exception"); + } + + // Defensive: if disconnect() bailed early (e.g. never connected), still + // tear down any state that may have leaked. if (subscription_thread_dispatcher_) { subscription_thread_dispatcher_->stopAll(); } @@ -86,13 +101,9 @@ Room::~Room() { const std::scoped_lock g(lock_); listener_to_remove = listener_id_; listener_id_ = 0; - // Move local participant out for cleanup outside the lock local_participant_to_cleanup = std::move(local_participant_); } - // Shutdown local participant (unregisters RPC handlers, etc.) before - // removing the listener. This prevents in-flight RPC responses from - // trying to use destroyed handles. if (local_participant_to_cleanup) { local_participant_to_cleanup->shutdown(); } @@ -100,8 +111,6 @@ Room::~Room() { if (listener_to_remove != 0) { FfiClient::instance().removeListener(listener_to_remove); } - - // local_participant_to_cleanup is destroyed here after listener is removed } void Room::setDelegate(RoomDelegate* delegate) { @@ -234,6 +243,83 @@ bool Room::Connect(const std::string& url, const std::string& token, const RoomO return connect(url, token, options); } +bool Room::disconnect(DisconnectReason reason) { + TRACE_EVENT0("livekit", "Room::disconnect"); + + std::shared_ptr handle; + RoomDelegate* delegate_snapshot = nullptr; + { + const std::scoped_lock g(lock_); + if (connection_state_ == ConnectionState::Disconnected) { + return false; + } + handle = room_handle_; + delegate_snapshot = delegate_; + // Flip state immediately so the in-flight Disconnected room-event we'll + // get back doesn't double-fire onDisconnected. Mirrors Python's + // Room.disconnect(), which also flips state before sending the request. + connection_state_ = ConnectionState::Disconnected; + } + + // Tell the FFI to close the room and wait for the callback. Catch the + // exception so we still run teardown below; the caller learns about the + // failure via the returned bool / logs. + bool ffi_ok = true; + if (handle) { + try { + FfiClient::instance().disconnectAsync(handle->get(), reason).get(); + } catch (const std::exception& e) { + LK_LOG_ERROR("Room::disconnect: FFI disconnect failed: {}", e.what()); + ffi_ok = false; + } + } + + // Stop dispatcher first so no track callbacks fire mid-teardown. + if (subscription_thread_dispatcher_) { + subscription_thread_dispatcher_->stopAll(); + } + + int listener_to_remove = 0; + std::unique_ptr local_participant_to_cleanup; + { + const std::scoped_lock g(lock_); + listener_to_remove = listener_id_; + listener_id_ = 0; + local_participant_to_cleanup = std::move(local_participant_); + remote_participants_.clear(); + room_handle_.reset(); + e2ee_manager_.reset(); + text_stream_readers_.clear(); + byte_stream_readers_.clear(); + } + + // Shut down local participant (unregisters RPC handlers, etc.) before + // removing the listener, so in-flight RPC responses don't reach a + // destroyed handle. + if (local_participant_to_cleanup) { + local_participant_to_cleanup->shutdown(); + } + + if (listener_to_remove != 0) { + FfiClient::instance().removeListener(listener_to_remove); + } + + // Fire onDisconnected exactly once, with the reason the caller passed. + if (delegate_snapshot) { + DisconnectedEvent ev; + ev.reason = reason; + try { + delegate_snapshot->onDisconnected(*this, ev); + } catch (const std::exception& e) { + LK_LOG_ERROR("Room::disconnect: onDisconnected threw: {}", e.what()); + } catch (...) { + LK_LOG_ERROR("Room::disconnect: onDisconnected threw: unknown exception"); + } + } + + return ffi_ok; +} + RoomInfoData Room::roomInfo() const { const std::scoped_lock g(lock_); return room_info_; @@ -1139,6 +1225,17 @@ void Room::onEvent(const FfiEvent& event) { break; } case proto::RoomEvent::kDisconnected: { + // If disconnect() was driven from our side, it already flipped state + // to Disconnected and fired the delegate; skip the duplicate here. + bool already_disconnected = false; + { + const std::scoped_lock guard(lock_); + already_disconnected = (connection_state_ == ConnectionState::Disconnected); + connection_state_ = ConnectionState::Disconnected; + } + if (already_disconnected) { + break; + } DisconnectedEvent ev; ev.reason = toDisconnectReason(re.disconnected().reason()); if (delegate_snapshot) { diff --git a/src/room_proto_converter.cpp b/src/room_proto_converter.cpp index 92918fa2..9fff5eb8 100644 --- a/src/room_proto_converter.cpp +++ b/src/room_proto_converter.cpp @@ -99,9 +99,80 @@ DataPacketKind toDataPacketKind(proto::DataPacketKind in) { } } -DisconnectReason toDisconnectReason(proto::DisconnectReason /*in*/) { - // TODO: map each proto::DisconnectReason to your DisconnectReason enum - return DisconnectReason::Unknown; +DisconnectReason toDisconnectReason(proto::DisconnectReason in) { + switch (in) { + case proto::CLIENT_INITIATED: + return DisconnectReason::ClientInitiated; + case proto::DUPLICATE_IDENTITY: + return DisconnectReason::DuplicateIdentity; + case proto::SERVER_SHUTDOWN: + return DisconnectReason::ServerShutdown; + case proto::PARTICIPANT_REMOVED: + return DisconnectReason::ParticipantRemoved; + case proto::ROOM_DELETED: + return DisconnectReason::RoomDeleted; + case proto::STATE_MISMATCH: + return DisconnectReason::StateMismatch; + case proto::JOIN_FAILURE: + return DisconnectReason::JoinFailure; + case proto::MIGRATION: + return DisconnectReason::Migration; + case proto::SIGNAL_CLOSE: + return DisconnectReason::SignalClose; + case proto::ROOM_CLOSED: + return DisconnectReason::RoomClosed; + case proto::USER_UNAVAILABLE: + return DisconnectReason::UserUnavailable; + case proto::USER_REJECTED: + return DisconnectReason::UserRejected; + case proto::SIP_TRUNK_FAILURE: + return DisconnectReason::SipTrunkFailure; + case proto::CONNECTION_TIMEOUT: + return DisconnectReason::ConnectionTimeout; + case proto::MEDIA_FAILURE: + return DisconnectReason::MediaFailure; + case proto::UNKNOWN_REASON: + default: + return DisconnectReason::Unknown; + } +} + +proto::DisconnectReason toProto(DisconnectReason in) { + switch (in) { + case DisconnectReason::ClientInitiated: + return proto::CLIENT_INITIATED; + case DisconnectReason::DuplicateIdentity: + return proto::DUPLICATE_IDENTITY; + case DisconnectReason::ServerShutdown: + return proto::SERVER_SHUTDOWN; + case DisconnectReason::ParticipantRemoved: + return proto::PARTICIPANT_REMOVED; + case DisconnectReason::RoomDeleted: + return proto::ROOM_DELETED; + case DisconnectReason::StateMismatch: + return proto::STATE_MISMATCH; + case DisconnectReason::JoinFailure: + return proto::JOIN_FAILURE; + case DisconnectReason::Migration: + return proto::MIGRATION; + case DisconnectReason::SignalClose: + return proto::SIGNAL_CLOSE; + case DisconnectReason::RoomClosed: + return proto::ROOM_CLOSED; + case DisconnectReason::UserUnavailable: + return proto::USER_UNAVAILABLE; + case DisconnectReason::UserRejected: + return proto::USER_REJECTED; + case DisconnectReason::SipTrunkFailure: + return proto::SIP_TRUNK_FAILURE; + case DisconnectReason::ConnectionTimeout: + return proto::CONNECTION_TIMEOUT; + case DisconnectReason::MediaFailure: + return proto::MEDIA_FAILURE; + case DisconnectReason::Unknown: + default: + return proto::UNKNOWN_REASON; + } } // --------- basic helper conversions --------- diff --git a/src/room_proto_converter.h b/src/room_proto_converter.h index 03d1c733..9b9d3d67 100644 --- a/src/room_proto_converter.h +++ b/src/room_proto_converter.h @@ -37,6 +37,7 @@ LIVEKIT_INTERNAL_API ConnectionQuality toConnectionQuality(proto::ConnectionQual LIVEKIT_INTERNAL_API ConnectionState toConnectionState(proto::ConnectionState in); LIVEKIT_INTERNAL_API DataPacketKind toDataPacketKind(proto::DataPacketKind in); LIVEKIT_INTERNAL_API DisconnectReason toDisconnectReason(proto::DisconnectReason in); +LIVEKIT_INTERNAL_API proto::DisconnectReason toProto(DisconnectReason in); LIVEKIT_INTERNAL_API UserPacketData fromProto(const proto::UserPacket& in); LIVEKIT_INTERNAL_API SipDtmfData fromProto(const proto::SipDTMF& in); diff --git a/src/tests/integration/test_room.cpp b/src/tests/integration/test_room.cpp index 9fb2490d..30378fa4 100644 --- a/src/tests/integration/test_room.cpp +++ b/src/tests/integration/test_room.cpp @@ -70,4 +70,66 @@ TEST_F(RoomTest, ConnectWithInvalidUrl) { EXPECT_FALSE(connected) << "Should fail to connect to invalid URL"; } +namespace { + +class DisconnectTrackingDelegate : public RoomDelegate { +public: + void onDisconnected(Room&, const DisconnectedEvent& ev) override { + ++count; + last_reason = ev.reason; + } + + std::atomic count{0}; + DisconnectReason last_reason = DisconnectReason::Unknown; +}; + +} // namespace + +// Issue #118: explicit disconnect() sends DisconnectRequest, flips state, +// and fires onDisconnected exactly once. +TEST_F(RoomTest, ExplicitDisconnectFiresDelegateAndClearsState) { + if (!server_available_) { + GTEST_SKIP() << "LIVEKIT_URL / LIVEKIT_TOKEN_A not set"; + } + + Room room; + DisconnectTrackingDelegate delegate; + room.setDelegate(&delegate); + + RoomOptions options; + ASSERT_TRUE(room.connect(server_url_, token_, options)) << "connect failed"; + ASSERT_EQ(room.connectionState(), ConnectionState::Connected); + ASSERT_NE(room.localParticipant(), nullptr); + + EXPECT_TRUE(room.disconnect()) << "disconnect should report success on a connected room"; + EXPECT_EQ(room.connectionState(), ConnectionState::Disconnected); + EXPECT_EQ(room.localParticipant(), nullptr) << "local participant should be cleared after disconnect"; + EXPECT_EQ(delegate.count.load(), 1) << "onDisconnected should fire exactly once"; + EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); + + // Idempotent: calling again on an already-disconnected room is a no-op. + EXPECT_FALSE(room.disconnect()) << "second disconnect should report no-op"; + EXPECT_EQ(delegate.count.load(), 1) << "delegate must not double-fire"; +} + +// Issue #118: destruction of a still-connected Room must trigger a graceful +// disconnect (RAII), not silently leak the participant on the server side. +TEST_F(RoomTest, DestructorTriggersGracefulDisconnect) { + if (!server_available_) { + GTEST_SKIP() << "LIVEKIT_URL / LIVEKIT_TOKEN_A not set"; + } + + DisconnectTrackingDelegate delegate; + { + Room room; + room.setDelegate(&delegate); + RoomOptions options; + ASSERT_TRUE(room.connect(server_url_, token_, options)); + ASSERT_EQ(room.connectionState(), ConnectionState::Connected); + // Let `room` go out of scope while still connected. + } + EXPECT_EQ(delegate.count.load(), 1) << "destructor should fire onDisconnected exactly once"; + EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); +} + } // namespace livekit::test From 377c8eac7d30c46a9caa32ebd018d7517df2cec9 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 29 May 2026 08:53:43 -0600 Subject: [PATCH 2/6] Maybe fix --- src/local_participant.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/local_participant.cpp b/src/local_participant.cpp index 7fda68ac..db404db2 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -29,6 +29,7 @@ #include "livekit/local_video_track.h" #include "livekit/room_delegate.h" #include "livekit/track.h" +#include "lk_log.h" #include "participant.pb.h" #include "room.pb.h" #include "room_proto_converter.h" @@ -439,7 +440,17 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s if (response_payload.has_value()) { msg->set_payload(*response_payload); } - FfiClient::instance().sendRequest(req); + // This is invoked on the FFI listener thread; an unhandled throw here would + // call std::terminate. The caller will observe a timeout instead. + try { + FfiClient::instance().sendRequest(req); + } catch (const std::exception& e) { + LK_LOG_ERROR("LocalParticipant: failed to send RPC invocation response (invocation_id={}): {}", invocation_id, + e.what()); + } catch (...) { + LK_LOG_ERROR("LocalParticipant: failed to send RPC invocation response (invocation_id={}): unknown exception", + invocation_id); + } } std::shared_ptr LocalParticipant::findTrackPublication(const std::string& sid) const { From 12946e28cbac6247eba4d361ed0dbe3960f43c18 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 29 May 2026 13:53:49 -0600 Subject: [PATCH 3/6] See if we can catch Rpc failure --- include/livekit/room.h | 4 ++-- src/ffi_client.h | 2 -- src/local_participant.cpp | 13 +---------- src/room.cpp | 3 +++ src/tests/integration/test_room.cpp | 36 +++++++++++------------------ src/tests/integration/test_rpc.cpp | 14 +++++++++++ 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/include/livekit/room.h b/include/livekit/room.h index b7e0ef1a..b02d8af7 100644 --- a/include/livekit/room.h +++ b/include/livekit/room.h @@ -141,14 +141,14 @@ class LIVEKIT_API Room { bool Connect(const std::string& url, const std::string& token, const RoomOptions& options); // NOLINTEND(readability-identifier-naming) - /// Gracefully disconnect from the room. + /// Disconnect from the room. /// /// Sends a `DisconnectRequest` to the server with the given reason, blocks /// until the FFI acknowledges, and tears down all room state (participants, /// listener, E2EE manager, subscription threads). The `onDisconnected` /// delegate is invoked once with the supplied reason. /// - /// Safe to call from any thread, but **must not** be called from inside a + /// @warning Safe to call from any thread, but **must not** be called from inside a /// `RoomDelegate` callback — doing so will deadlock the event listener. /// /// @note `~Room()` invokes `disconnect()` automatically if the room is diff --git a/src/ffi_client.h b/src/ffi_client.h index f595cb98..5136e49e 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -95,8 +95,6 @@ class LIVEKIT_INTERNAL_API FfiClient { std::future connectAsync(const std::string& url, const std::string& token, const RoomOptions& options); - // Send a DisconnectRequest for the given room handle and wait for the - // FFI's DisconnectCallback. Throws std::runtime_error on FFI failure. std::future disconnectAsync(uintptr_t room_handle, DisconnectReason reason); // Track APIs diff --git a/src/local_participant.cpp b/src/local_participant.cpp index db404db2..7fda68ac 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -29,7 +29,6 @@ #include "livekit/local_video_track.h" #include "livekit/room_delegate.h" #include "livekit/track.h" -#include "lk_log.h" #include "participant.pb.h" #include "room.pb.h" #include "room_proto_converter.h" @@ -440,17 +439,7 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s if (response_payload.has_value()) { msg->set_payload(*response_payload); } - // This is invoked on the FFI listener thread; an unhandled throw here would - // call std::terminate. The caller will observe a timeout instead. - try { - FfiClient::instance().sendRequest(req); - } catch (const std::exception& e) { - LK_LOG_ERROR("LocalParticipant: failed to send RPC invocation response (invocation_id={}): {}", invocation_id, - e.what()); - } catch (...) { - LK_LOG_ERROR("LocalParticipant: failed to send RPC invocation response (invocation_id={}): unknown exception", - invocation_id); - } + FfiClient::instance().sendRequest(req); } std::shared_ptr LocalParticipant::findTrackPublication(const std::string& sid) const { diff --git a/src/room.cpp b/src/room.cpp index dde2b46d..61fe4eeb 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -246,6 +246,9 @@ bool Room::Connect(const std::string& url, const std::string& token, const RoomO bool Room::disconnect(DisconnectReason reason) { TRACE_EVENT0("livekit", "Room::disconnect"); + // Hold onto this in case the + auto prev_connection_state = connection_state_; + std::shared_ptr handle; RoomDelegate* delegate_snapshot = nullptr; { diff --git a/src/tests/integration/test_room.cpp b/src/tests/integration/test_room.cpp index 30378fa4..15e5d780 100644 --- a/src/tests/integration/test_room.cpp +++ b/src/tests/integration/test_room.cpp @@ -85,13 +85,8 @@ class DisconnectTrackingDelegate : public RoomDelegate { } // namespace -// Issue #118: explicit disconnect() sends DisconnectRequest, flips state, -// and fires onDisconnected exactly once. -TEST_F(RoomTest, ExplicitDisconnectFiresDelegateAndClearsState) { - if (!server_available_) { - GTEST_SKIP() << "LIVEKIT_URL / LIVEKIT_TOKEN_A not set"; - } - +// Case: User calls disconnect() +TEST_F(RoomTest, UserDisconnect) { Room room; DisconnectTrackingDelegate delegate; room.setDelegate(&delegate); @@ -107,27 +102,24 @@ TEST_F(RoomTest, ExplicitDisconnectFiresDelegateAndClearsState) { EXPECT_EQ(delegate.count.load(), 1) << "onDisconnected should fire exactly once"; EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); - // Idempotent: calling again on an already-disconnected room is a no-op. + // Calling again on an already-disconnected room is a no-op EXPECT_FALSE(room.disconnect()) << "second disconnect should report no-op"; EXPECT_EQ(delegate.count.load(), 1) << "delegate must not double-fire"; } -// Issue #118: destruction of a still-connected Room must trigger a graceful -// disconnect (RAII), not silently leak the participant on the server side. -TEST_F(RoomTest, DestructorTriggersGracefulDisconnect) { - if (!server_available_) { - GTEST_SKIP() << "LIVEKIT_URL / LIVEKIT_TOKEN_A not set"; - } +// Case: Room goes out of scope while still connected +TEST_F(RoomTest, RoomDestructorDisconnect) { + std::unique_ptr room = std::make_unique(); DisconnectTrackingDelegate delegate; - { - Room room; - room.setDelegate(&delegate); - RoomOptions options; - ASSERT_TRUE(room.connect(server_url_, token_, options)); - ASSERT_EQ(room.connectionState(), ConnectionState::Connected); - // Let `room` go out of scope while still connected. - } + room->setDelegate(&delegate); + RoomOptions options; + ASSERT_TRUE(room->connect(server_url_, token_, options)); + ASSERT_EQ(room->connectionState(), ConnectionState::Connected); + + room.reset(); + + // Let room go out of scope while still connected EXPECT_EQ(delegate.count.load(), 1) << "destructor should fire onDisconnected exactly once"; EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); } diff --git a/src/tests/integration/test_rpc.cpp b/src/tests/integration/test_rpc.cpp index 18353244..39d45293 100644 --- a/src/tests/integration/test_rpc.cpp +++ b/src/tests/integration/test_rpc.cpp @@ -374,6 +374,17 @@ TEST_F(RpcIntegrationTest, ConcurrentRpcCalls) { receiver_room.reset(); } +class DummyRoomDelegate : public RoomDelegate { + void onDisconnected(Room& room, const DisconnectedEvent& disconnected_event) override { + std::cout << "Room disconnected\n"; + std::cout << "Room name: " << room.roomInfo().name << "\n"; + std::cout << "Room sid: " << room.roomInfo().sid.value_or("") << "\n"; + std::cout << "Reason: " + << (disconnected_event.reason == DisconnectReason::ClientInitiated ? "ClientInitiated" : "Unknown") + << "\n"; + } +}; + // Integration test: Run for approximately 1 minute TEST_F(RpcIntegrationTest, OneMinuteIntegration) { if (!config_.available) { @@ -381,6 +392,8 @@ TEST_F(RpcIntegrationTest, OneMinuteIntegration) { } auto receiver_room = std::make_unique(); + DummyRoomDelegate delegate; + receiver_room->setDelegate(&delegate); RoomOptions options; options.auto_subscribe = true; @@ -400,6 +413,7 @@ TEST_F(RpcIntegrationTest, OneMinuteIntegration) { }); auto caller_room = std::make_unique(); + caller_room->setDelegate(&delegate); bool caller_connected = caller_room->connect(config_.url, config_.token_a, options); ASSERT_TRUE(caller_connected) << "Caller failed to connect"; From dd91cd9e18a1e0ae81ac4b8b72d4e336d6bb0c40 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 29 May 2026 14:36:59 -0600 Subject: [PATCH 4/6] Drain RPC invocations on kEos + add lifecycle tracing The kEos RoomEvent handler moves out and destroys the local participant without first calling shutdown(). If the listener thread is mid-RPC- invocation when kEos arrives, the participant's FfiHandle is dropped while a handler is still in flight; the handler's later sendRequest then hits an invalid Rust handle and the uncaught std::runtime_error on the listener thread aborts the process. Mirror the ordering used in disconnect(): call shutdown() on the moved-out participant before letting it destruct. Add INFO-level tracing at the key lifecycle points (~Room, disconnect, kDisconnected, kEos, FfiHandle drops, INVALID_HANDLE error path) and DEBUG tracing on the RPC invocation hot path. Bump RUST_LOG in the integration-test CI step so the Rust side's "handle not found" error message lands in the log. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/tests.yml | 6 +++++- src/ffi_client.cpp | 4 ++++ src/ffi_handle.cpp | 2 ++ src/local_participant.cpp | 13 ++++++++++++- src/room.cpp | 16 ++++++++++++++++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b998be8f..5381c547 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -261,7 +261,11 @@ jobs: timeout-minutes: 5 shell: bash env: - RUST_LOG: "metrics=debug" + # Bump livekit_ffi to debug so handle-table errors (the exact Rust + # message that produces our INVALID_HANDLE → terminate path) land + # in the CI log. Keep livekit at info to avoid drowning out the + # interesting lines. + RUST_LOG: "metrics=debug,livekit_ffi=debug,livekit=info" run: | set -euo pipefail source .token_helpers/set_data_track_test_tokens.bash diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 1ae3ddf4..44d1f00f 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -202,6 +202,10 @@ proto::FfiResponse FfiClient::sendRequest(const proto::FfiRequest& request) cons const FfiHandleId handle = livekit_ffi_request(reinterpret_cast(bytes.data()), bytes.size(), &resp_ptr, &resp_len); if (handle == INVALID_HANDLE) { + LK_LOG_ERROR( + "FfiClient::sendRequest: livekit_ffi_request returned INVALID_HANDLE; " + "request.message_case={}, bytes_len={}", + static_cast(request.message_case()), bytes.size()); throw std::runtime_error("failed to send request, received an invalid handle"); } diff --git a/src/ffi_handle.cpp b/src/ffi_handle.cpp index b19ebe94..3885d79b 100644 --- a/src/ffi_handle.cpp +++ b/src/ffi_handle.cpp @@ -17,6 +17,7 @@ #include "livekit/ffi_handle.h" #include "livekit_ffi.h" +#include "lk_log.h" namespace livekit { @@ -35,6 +36,7 @@ FfiHandle& FfiHandle::operator=(FfiHandle&& other) noexcept { void FfiHandle::reset(uintptr_t new_handle) noexcept { if (handle_) { + LK_LOG_INFO("FfiHandle::reset: dropping handle {} (replacement={})", handle_, new_handle); livekit_ffi_drop_handle(handle_); } handle_ = new_handle; diff --git a/src/local_participant.cpp b/src/local_participant.cpp index 7fda68ac..7ba99af5 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -370,6 +370,8 @@ void LocalParticipant::shutdown() { void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const std::string& method, const std::string& request_id, const std::string& caller_identity, const std::string& payload, double response_timeout_sec) { + LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: entry (handle={}, invocation_id={}, method={})", + ffiHandleId(), invocation_id, method); // Capture shared state so it outlives LocalParticipant if needed auto state = rpc_state_; @@ -377,6 +379,8 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s { const std::scoped_lock lock(state->mutex); if (state->shutting_down) { + LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: shutting_down, skipping (invocation_id={})", + invocation_id); // Already shutting down, don't process new invocations return; } @@ -423,6 +427,10 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s { const std::scoped_lock lock(state->mutex); if (state->shutting_down) { + LK_LOG_DEBUG( + "LocalParticipant::handleRpcMethodInvocation: shutdown during handler, skipping response " + "(invocation_id={})", + invocation_id); // Shutdown started, don't send response - handle may be invalid return; } @@ -430,7 +438,8 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s FfiRequest req; auto* msg = req.mutable_rpc_method_invocation_response(); - msg->set_local_participant_handle(ffiHandleId()); + const auto handle_id_at_send = ffiHandleId(); + msg->set_local_participant_handle(handle_id_at_send); msg->set_invocation_id(invocation_id); if (response_error.has_value()) { auto* err_proto = msg->mutable_error(); @@ -439,6 +448,8 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s if (response_payload.has_value()) { msg->set_payload(*response_payload); } + LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: sending response (handle={}, invocation_id={})", + handle_id_at_send, invocation_id); FfiClient::instance().sendRequest(req); } diff --git a/src/room.cpp b/src/room.cpp index 61fe4eeb..5154340c 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -76,6 +76,7 @@ void readyForRoomEvent(std::uint64_t room_handle) { Room::Room() : subscription_thread_dispatcher_(std::make_unique()) {} Room::~Room() { + LK_LOG_INFO("Room::~Room: entry (this={})", static_cast(this)); // Issue a graceful disconnect so the server sees us leave instead of // timing out (RAII expectation; see issue #118). disconnect() does the // full teardown including subscription threads, listener, and local @@ -245,6 +246,7 @@ bool Room::Connect(const std::string& url, const std::string& token, const RoomO bool Room::disconnect(DisconnectReason reason) { TRACE_EVENT0("livekit", "Room::disconnect"); + LK_LOG_INFO("Room::disconnect: entry (this={}, reason={})", static_cast(this), static_cast(reason)); // Hold onto this in case the auto prev_connection_state = connection_state_; @@ -254,6 +256,7 @@ bool Room::disconnect(DisconnectReason reason) { { const std::scoped_lock g(lock_); if (connection_state_ == ConnectionState::Disconnected) { + LK_LOG_INFO("Room::disconnect: already disconnected, returning false (this={})", static_cast(this)); return false; } handle = room_handle_; @@ -1228,6 +1231,8 @@ void Room::onEvent(const FfiEvent& event) { break; } case proto::RoomEvent::kDisconnected: { + LK_LOG_INFO("Room::onFfiEvent: kDisconnected received (this={}, reason={})", static_cast(this), + static_cast(re.disconnected().reason())); // If disconnect() was driven from our side, it already flipped state // to Disconnected and fired the delegate; skip the duplicate here. bool already_disconnected = false; @@ -1261,6 +1266,7 @@ void Room::onEvent(const FfiEvent& event) { break; } case proto::RoomEvent::kEos: { + LK_LOG_INFO("Room::onFfiEvent: kEos received (this={})", static_cast(this)); if (subscription_thread_dispatcher_) { subscription_thread_dispatcher_->stopAll(); } @@ -1293,6 +1299,16 @@ void Room::onEvent(const FfiEvent& event) { old_byte_readers = std::move(byte_stream_readers_); } + // Drain in-flight RPC invocations before destroying the local + // participant's FFI handle. Mirrors the ordering in disconnect(); + // without this, a listener-thread RPC handler can race with handle + // disposal and send to a dead handle → INVALID_HANDLE → terminate. + if (old_local_participant) { + LK_LOG_INFO("Room::onFfiEvent: kEos shutting down local participant (handle={})", + old_local_participant->ffiHandleId()); + old_local_participant->shutdown(); + } + // Remove listener outside lock if (listener_to_remove != 0) { FfiClient::instance().removeListener(listener_to_remove); From 6159bbc4bca433929b8ae12e3c177ecb7bd8e2bc Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 29 May 2026 16:44:14 -0600 Subject: [PATCH 5/6] Cleanup, shutdown local participants before --- .github/workflows/tests.yml | 6 +- src/ffi_client.cpp | 4 - src/ffi_handle.cpp | 2 - src/local_participant.cpp | 13 +--- src/room.cpp | 115 ++++++++++++---------------- src/tests/integration/test_room.cpp | 9 +-- 6 files changed, 56 insertions(+), 93 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5381c547..b998be8f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -261,11 +261,7 @@ jobs: timeout-minutes: 5 shell: bash env: - # Bump livekit_ffi to debug so handle-table errors (the exact Rust - # message that produces our INVALID_HANDLE → terminate path) land - # in the CI log. Keep livekit at info to avoid drowning out the - # interesting lines. - RUST_LOG: "metrics=debug,livekit_ffi=debug,livekit=info" + RUST_LOG: "metrics=debug" run: | set -euo pipefail source .token_helpers/set_data_track_test_tokens.bash diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 44d1f00f..1ae3ddf4 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -202,10 +202,6 @@ proto::FfiResponse FfiClient::sendRequest(const proto::FfiRequest& request) cons const FfiHandleId handle = livekit_ffi_request(reinterpret_cast(bytes.data()), bytes.size(), &resp_ptr, &resp_len); if (handle == INVALID_HANDLE) { - LK_LOG_ERROR( - "FfiClient::sendRequest: livekit_ffi_request returned INVALID_HANDLE; " - "request.message_case={}, bytes_len={}", - static_cast(request.message_case()), bytes.size()); throw std::runtime_error("failed to send request, received an invalid handle"); } diff --git a/src/ffi_handle.cpp b/src/ffi_handle.cpp index 3885d79b..b19ebe94 100644 --- a/src/ffi_handle.cpp +++ b/src/ffi_handle.cpp @@ -17,7 +17,6 @@ #include "livekit/ffi_handle.h" #include "livekit_ffi.h" -#include "lk_log.h" namespace livekit { @@ -36,7 +35,6 @@ FfiHandle& FfiHandle::operator=(FfiHandle&& other) noexcept { void FfiHandle::reset(uintptr_t new_handle) noexcept { if (handle_) { - LK_LOG_INFO("FfiHandle::reset: dropping handle {} (replacement={})", handle_, new_handle); livekit_ffi_drop_handle(handle_); } handle_ = new_handle; diff --git a/src/local_participant.cpp b/src/local_participant.cpp index 7ba99af5..7fda68ac 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -370,8 +370,6 @@ void LocalParticipant::shutdown() { void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const std::string& method, const std::string& request_id, const std::string& caller_identity, const std::string& payload, double response_timeout_sec) { - LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: entry (handle={}, invocation_id={}, method={})", - ffiHandleId(), invocation_id, method); // Capture shared state so it outlives LocalParticipant if needed auto state = rpc_state_; @@ -379,8 +377,6 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s { const std::scoped_lock lock(state->mutex); if (state->shutting_down) { - LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: shutting_down, skipping (invocation_id={})", - invocation_id); // Already shutting down, don't process new invocations return; } @@ -427,10 +423,6 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s { const std::scoped_lock lock(state->mutex); if (state->shutting_down) { - LK_LOG_DEBUG( - "LocalParticipant::handleRpcMethodInvocation: shutdown during handler, skipping response " - "(invocation_id={})", - invocation_id); // Shutdown started, don't send response - handle may be invalid return; } @@ -438,8 +430,7 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s FfiRequest req; auto* msg = req.mutable_rpc_method_invocation_response(); - const auto handle_id_at_send = ffiHandleId(); - msg->set_local_participant_handle(handle_id_at_send); + msg->set_local_participant_handle(ffiHandleId()); msg->set_invocation_id(invocation_id); if (response_error.has_value()) { auto* err_proto = msg->mutable_error(); @@ -448,8 +439,6 @@ void LocalParticipant::handleRpcMethodInvocation(uint64_t invocation_id, const s if (response_payload.has_value()) { msg->set_payload(*response_payload); } - LK_LOG_DEBUG("LocalParticipant::handleRpcMethodInvocation: sending response (handle={}, invocation_id={})", - handle_id_at_send, invocation_id); FfiClient::instance().sendRequest(req); } diff --git a/src/room.cpp b/src/room.cpp index 5154340c..2f854ee5 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -76,42 +76,17 @@ void readyForRoomEvent(std::uint64_t room_handle) { Room::Room() : subscription_thread_dispatcher_(std::make_unique()) {} Room::~Room() { - LK_LOG_INFO("Room::~Room: entry (this={})", static_cast(this)); - // Issue a graceful disconnect so the server sees us leave instead of - // timing out (RAII expectation; see issue #118). disconnect() does the - // full teardown including subscription threads, listener, and local - // participant, so the destructor only needs to handle the - // already-disconnected path. + // disconnect() is used for all tear down cases: it handles the + // already-disconnected case (returns false, no-op), the partial/Reconnecting + // case, and the FFI-failure case (local teardown still runs). Nothing else + // needs to live in the destructor. try { - disconnect(); + (void)disconnect(); // Don't need return value } catch (const std::exception& e) { LK_LOG_ERROR("Room::~Room: graceful disconnect failed: {}", e.what()); } catch (...) { LK_LOG_ERROR("Room::~Room: graceful disconnect failed: unknown exception"); } - - // Defensive: if disconnect() bailed early (e.g. never connected), still - // tear down any state that may have leaked. - if (subscription_thread_dispatcher_) { - subscription_thread_dispatcher_->stopAll(); - } - - int listener_to_remove = 0; - std::unique_ptr local_participant_to_cleanup; - { - const std::scoped_lock g(lock_); - listener_to_remove = listener_id_; - listener_id_ = 0; - local_participant_to_cleanup = std::move(local_participant_); - } - - if (local_participant_to_cleanup) { - local_participant_to_cleanup->shutdown(); - } - - if (listener_to_remove != 0) { - FfiClient::instance().removeListener(listener_to_remove); - } } void Room::setDelegate(RoomDelegate* delegate) { @@ -246,66 +221,79 @@ bool Room::Connect(const std::string& url, const std::string& token, const RoomO bool Room::disconnect(DisconnectReason reason) { TRACE_EVENT0("livekit", "Room::disconnect"); - LK_LOG_INFO("Room::disconnect: entry (this={}, reason={})", static_cast(this), static_cast(reason)); - // Hold onto this in case the - auto prev_connection_state = connection_state_; + // Canonical teardown path. Move all owned state out under the lock, then + // operate on it outside the lock. The destructor (and any caller) gets + // the same behavior: once this returns, the Room is fully torn down. + // + // Return value: + // true - we owned live state and tore it down (FFI disconnect succeeded) + // false - either already disconnected (no-op) or FFI disconnect failed. + // In both false cases local-side teardown still completed. std::shared_ptr handle; RoomDelegate* delegate_snapshot = nullptr; + std::unique_ptr local_participant_to_cleanup; + std::unordered_map> remote_participants_to_clear; + std::unique_ptr e2ee_manager_to_clear; + std::unordered_map> text_stream_readers_to_clear; + std::unordered_map> byte_stream_readers_to_clear; + int listener_to_remove = 0; + { const std::scoped_lock g(lock_); if (connection_state_ == ConnectionState::Disconnected) { - LK_LOG_INFO("Room::disconnect: already disconnected, returning false (this={})", static_cast(this)); + // Already torn down (or never connected). Nothing to do. return false; } handle = room_handle_; delegate_snapshot = delegate_; + // Take ownership of everything under the lock so the kEos handler (which + // also tries to move it out) loses any race here — only one teardown + // path operates on this state. + local_participant_to_cleanup = std::move(local_participant_); + remote_participants_to_clear = std::move(remote_participants_); + e2ee_manager_to_clear = std::move(e2ee_manager_); + text_stream_readers_to_clear = std::move(text_stream_readers_); + byte_stream_readers_to_clear = std::move(byte_stream_readers_); + listener_to_remove = listener_id_; + listener_id_ = 0; + room_handle_.reset(); // Flip state immediately so the in-flight Disconnected room-event we'll // get back doesn't double-fire onDisconnected. Mirrors Python's // Room.disconnect(), which also flips state before sending the request. connection_state_ = ConnectionState::Disconnected; } - // Tell the FFI to close the room and wait for the callback. Catch the - // exception so we still run teardown below; the caller learns about the - // failure via the returned bool / logs. + // Drain in-flight RPC handlers BEFORE telling Rust to tear down the room. + // Mirrors client-sdk-python's Room.disconnect() ordering: once the FFI + // dispatches the Disconnect, Rust starts invalidating participant handles + // in its table, and any listener-thread RPC handler still mid-flight + // would race with that invalidation and send to a dead handle → + // INVALID_HANDLE → terminate. + if (local_participant_to_cleanup) { + local_participant_to_cleanup->shutdown(); + } + + // Tell the FFI to close the room and wait for the callback. If this fails + // we still complete local-side teardown below — releasing the listener, + // dropping handles, and notifying the delegate — so the Room is fully + // cleaned up regardless of whether the FFI round-trip succeeded. bool ffi_ok = true; if (handle) { try { FfiClient::instance().disconnectAsync(handle->get(), reason).get(); } catch (const std::exception& e) { - LK_LOG_ERROR("Room::disconnect: FFI disconnect failed: {}", e.what()); + LK_LOG_ERROR("Room::disconnect: FFI disconnect failed (continuing local teardown): {}", e.what()); ffi_ok = false; } } - // Stop dispatcher first so no track callbacks fire mid-teardown. + // Stop dispatcher so no track callbacks fire mid-teardown. if (subscription_thread_dispatcher_) { subscription_thread_dispatcher_->stopAll(); } - int listener_to_remove = 0; - std::unique_ptr local_participant_to_cleanup; - { - const std::scoped_lock g(lock_); - listener_to_remove = listener_id_; - listener_id_ = 0; - local_participant_to_cleanup = std::move(local_participant_); - remote_participants_.clear(); - room_handle_.reset(); - e2ee_manager_.reset(); - text_stream_readers_.clear(); - byte_stream_readers_.clear(); - } - - // Shut down local participant (unregisters RPC handlers, etc.) before - // removing the listener, so in-flight RPC responses don't reach a - // destroyed handle. - if (local_participant_to_cleanup) { - local_participant_to_cleanup->shutdown(); - } - if (listener_to_remove != 0) { FfiClient::instance().removeListener(listener_to_remove); } @@ -323,6 +311,8 @@ bool Room::disconnect(DisconnectReason reason) { } } + // Moved-out state (local participant, remote participants, e2ee manager, + // stream readers) destructs here, releasing FFI handles. return ffi_ok; } @@ -1231,8 +1221,6 @@ void Room::onEvent(const FfiEvent& event) { break; } case proto::RoomEvent::kDisconnected: { - LK_LOG_INFO("Room::onFfiEvent: kDisconnected received (this={}, reason={})", static_cast(this), - static_cast(re.disconnected().reason())); // If disconnect() was driven from our side, it already flipped state // to Disconnected and fired the delegate; skip the duplicate here. bool already_disconnected = false; @@ -1266,7 +1254,6 @@ void Room::onEvent(const FfiEvent& event) { break; } case proto::RoomEvent::kEos: { - LK_LOG_INFO("Room::onFfiEvent: kEos received (this={})", static_cast(this)); if (subscription_thread_dispatcher_) { subscription_thread_dispatcher_->stopAll(); } @@ -1304,8 +1291,6 @@ void Room::onEvent(const FfiEvent& event) { // without this, a listener-thread RPC handler can race with handle // disposal and send to a dead handle → INVALID_HANDLE → terminate. if (old_local_participant) { - LK_LOG_INFO("Room::onFfiEvent: kEos shutting down local participant (handle={})", - old_local_participant->ffiHandleId()); old_local_participant->shutdown(); } diff --git a/src/tests/integration/test_room.cpp b/src/tests/integration/test_room.cpp index 15e5d780..5dbd68c6 100644 --- a/src/tests/integration/test_room.cpp +++ b/src/tests/integration/test_room.cpp @@ -96,19 +96,19 @@ TEST_F(RoomTest, UserDisconnect) { ASSERT_EQ(room.connectionState(), ConnectionState::Connected); ASSERT_NE(room.localParticipant(), nullptr); - EXPECT_TRUE(room.disconnect()) << "disconnect should report success on a connected room"; + EXPECT_NO_THROW(room.disconnect()) << "disconnect should not throw on a connected room"; EXPECT_EQ(room.connectionState(), ConnectionState::Disconnected); EXPECT_EQ(room.localParticipant(), nullptr) << "local participant should be cleared after disconnect"; EXPECT_EQ(delegate.count.load(), 1) << "onDisconnected should fire exactly once"; EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); // Calling again on an already-disconnected room is a no-op - EXPECT_FALSE(room.disconnect()) << "second disconnect should report no-op"; + EXPECT_NO_THROW(room.disconnect()) << "second disconnect should not throw on an already-disconnected room"; EXPECT_EQ(delegate.count.load(), 1) << "delegate must not double-fire"; } // Case: Room goes out of scope while still connected -TEST_F(RoomTest, RoomDestructorDisconnect) { +TEST_F(RoomTest, DestructorDisconnect) { std::unique_ptr room = std::make_unique(); DisconnectTrackingDelegate delegate; @@ -117,9 +117,8 @@ TEST_F(RoomTest, RoomDestructorDisconnect) { ASSERT_TRUE(room->connect(server_url_, token_, options)); ASSERT_EQ(room->connectionState(), ConnectionState::Connected); - room.reset(); + room.reset(); // invokes destructor which calls disconnect() - // Let room go out of scope while still connected EXPECT_EQ(delegate.count.load(), 1) << "destructor should fire onDisconnected exactly once"; EXPECT_EQ(delegate.last_reason, DisconnectReason::ClientInitiated); } From e3e7e15387305ef51514cebe20b638f963cf2249 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 29 May 2026 16:47:47 -0600 Subject: [PATCH 6/6] Additional cleanup --- src/tests/integration/test_rpc.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/tests/integration/test_rpc.cpp b/src/tests/integration/test_rpc.cpp index 39d45293..18353244 100644 --- a/src/tests/integration/test_rpc.cpp +++ b/src/tests/integration/test_rpc.cpp @@ -374,17 +374,6 @@ TEST_F(RpcIntegrationTest, ConcurrentRpcCalls) { receiver_room.reset(); } -class DummyRoomDelegate : public RoomDelegate { - void onDisconnected(Room& room, const DisconnectedEvent& disconnected_event) override { - std::cout << "Room disconnected\n"; - std::cout << "Room name: " << room.roomInfo().name << "\n"; - std::cout << "Room sid: " << room.roomInfo().sid.value_or("") << "\n"; - std::cout << "Reason: " - << (disconnected_event.reason == DisconnectReason::ClientInitiated ? "ClientInitiated" : "Unknown") - << "\n"; - } -}; - // Integration test: Run for approximately 1 minute TEST_F(RpcIntegrationTest, OneMinuteIntegration) { if (!config_.available) { @@ -392,8 +381,6 @@ TEST_F(RpcIntegrationTest, OneMinuteIntegration) { } auto receiver_room = std::make_unique(); - DummyRoomDelegate delegate; - receiver_room->setDelegate(&delegate); RoomOptions options; options.auto_subscribe = true; @@ -413,7 +400,6 @@ TEST_F(RpcIntegrationTest, OneMinuteIntegration) { }); auto caller_room = std::make_unique(); - caller_room->setDelegate(&delegate); bool caller_connected = caller_room->connect(config_.url, config_.token_a, options); ASSERT_TRUE(caller_connected) << "Caller failed to connect";