Bugfix: Server-initiated room disconnect shutdown fix by alan-george-lk · Pull Request #205 · livekit/client-sdk-cpp

alan-george-lk · 2026-07-13T17:42:48Z

Summary

Fixes server-initiated room disconnect shutdown by routing explicit disconnect, destructor cleanup, server disconnect events, and EOS through a shared shutdown() helper, which by proxy cleans up event code
Ensure server Disconnected events remove the C++ FFI listener, stop subscriptions, clear room-owned state, and fire onDisconnected exactly once without re-sending an FFI disconnect

Before fix, when server deleted the room:

[2026-07-13 10:02:31.515] [livekit] [error] FfiClient listener threw: mutex lock failed: Invalid argument

Above observed on Mac, but crashed on Linux (user-reported). No longer after the fix.

Testing

Original issue reproduced via local binary (not committed), then fixed and confirmed no longer occurred
Add regression coverage for synthetic FFI server disconnects and local-SFU room deletion via lk --dev room delete
Consolidates emitFfiEvent into standalone header helper ffi_utils.h for use across two tests now

Ticket

BOT-464

Co-authored-by: Cursor <cursoragent@cursor.com>

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

xianshijing-lk

lgtm, assuming you address those comments.

xianshijing-lk · 2026-07-14T03:04:10Z

+    const bool has_room_state = connection_state_ != ConnectionState::Disconnected || listener_id_ != 0 ||
+                                room_handle_ || local_participant_ || !remote_participants_.empty();
+    if (teardown_started_ || !has_room_state) {
      return false;


should we return true if nothing to do or it is being torndown ?

I'm open to changing this, right now our public API for disconnect() states:

@returns true if the graceful disconnect succeeds; false if the room was already disconnected (no-op) or the graceful disconnect fails.

Which this matches, false == noop. I get how that could be confusing, but I think the idea is true is when the disconnect actually happened

That is fair. We can keep the current behavior to avoid breaking changes.

FWIW we have a public connectionState so apps can check the state before trying to disconnect

…gfix-room-shutdown

Align the shared cleanup helper with LocalParticipant/FfiClient naming, keep disconnect() returning false for no-ops per its documented contract, and cover Room reuse after server-initiated shutdown. Co-authored-by: Cursor <cursoragent@cursor.com>

xianshijing-lk · 2026-07-14T04:09:14Z

lgtm, thanks

devin-ai-integration

Devin Review found 2 new potential issues.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-07-14T04:15:25Z

        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<std::mutex> 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) {
-            delegate_snapshot->onDisconnected(*this, ev);
-          }
+          const auto reason = toDisconnectReason(re.disconnected().reason());
+          (void)shutdown(false, reason, true);
          break;


🔴 Server-initiated disconnect silently suppresses the end-of-stream callback to the application

The listener is removed during server-disconnect cleanup (shutdown(false, reason, true) at src/room.cpp:1199) before the Rust FFI sends the follow-up end-of-stream event, so the onRoomEos delegate callback is never delivered.

Impact: Applications relying on the end-of-stream callback for final cleanup after a server-initiated disconnect will silently miss it.

Mechanism: kDisconnected removes the listener before kEos arrives

In the old code, kDisconnected (at the old src/room.cpp:1170–1186) only set connection_state_ to Disconnected and fired onDisconnected. The FFI listener remained registered, so the subsequent kEos event from Rust would still be dispatched to Room::onEvent(), which performed full state cleanup and called delegate_snapshot->onRoomEos().

In the new code, kDisconnected calls shutdown(false, reason, true) which:

Moves out all room state (src/room.cpp:239–248)

Removes the FFI listener via FfiClient::instance().removeListener() (src/room.cpp:290)

Fires onDisconnected (src/room.cpp:311)

Because the listener is now removed, when Rust later sends the kEos event, FfiClient::pushEvent finds no matching listener slot for this Room. The kEos case at src/room.cpp:1223–1230 is never reached, and onRoomEos is never called.

The old event sequence for server disconnect was: kDisconnected → onDisconnected → kEos → cleanup + onRoomEos.
The new sequence is: kDisconnected → cleanup + onDisconnected → (kEos silently dropped).

Was this helpful? React with 👍 or 👎 to provide feedback.

hard to image apps would be using kEos rather than kDisconnected, i guess something to consider

stephen-derosa · 2026-07-14T16:00:26Z

-          }
-
-          // old_* state is destroyed here when going out of scope
+          (void)shutdown(false, DisconnectReason::Unknown, false);


is EOS not a DisconnectReason?

TODO: Confirm the flow is this on server disconnect:

Server disconnect event (do shutdown)

EOS (don't need shutdown)

Verify that the EOS is double-calling shutdown right now as written (after server close)

stephen-derosa

nice lgtm!

…gfix-room-shutdown

devin-ai-integration

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

devin-ai-integration · 2026-07-15T16:54:58Z

        case proto::RoomEvent::kEos: {
-          if (subscription_thread_dispatcher_) {
-            subscription_thread_dispatcher_->stopAll();
-          }
-
-          int listener_to_remove = 0;
-
-          // Move state out of lock scope before destroying to avoid holding lock
-          // during potentially long destructors
-          std::shared_ptr<LocalParticipant> old_local_participant;
-          std::unordered_map<std::string, std::shared_ptr<RemoteParticipant>> old_remote_participants;
-          std::shared_ptr<FfiHandle> old_room_handle;
-          std::shared_ptr<E2EEManager> old_e2ee_manager;
-          std::unordered_map<std::string, std::shared_ptr<TextStreamReader>> old_text_readers;
-          std::unordered_map<std::string, std::shared_ptr<ByteStreamReader>> old_byte_readers;
-
-          {
-            const std::scoped_lock<std::mutex> guard(lock_);
-            listener_to_remove = listener_id_;
-            listener_id_ = 0;
-
-            // Reset connection state
-            connection_state_ = ConnectionState::Disconnected;
-
-            // Move state out for cleanup outside lock
-            old_local_participant = std::move(local_participant_);
-            old_remote_participants = std::move(remote_participants_);
-            old_room_handle = std::move(room_handle_);
-            old_e2ee_manager = std::move(e2ee_manager_);
-            old_text_readers = std::move(text_stream_readers_);
-            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) {
-            old_local_participant->shutdown();
-          }
-
-          // Remove listener outside lock
-          if (listener_to_remove != 0) {
-            FfiClient::instance().removeListener(listener_to_remove);
-          }
-
-          if (old_local_participant) {
-            old_local_participant->shutdown();
-          }
-
-          // old_* state is destroyed here when going out of scope
-
          const RoomEosEvent ev;
          if (delegate_snapshot) {
            delegate_snapshot->onRoomEos(*this, ev);


🔴 End-of-stream event no longer cleans up room resources, causing leaks when no prior disconnect event arrives

All teardown logic was removed from the end-of-stream handler (kEos at src/room.cpp:1221), so if the stream ends without a preceding disconnect event, the room's internal state (FFI handles, participants, listener registration) is never released.

Impact: The room leaks FFI resources and its event listener, and becomes permanently stuck in a stale state that cannot be recovered or reconnected.

Mechanism: kEos teardown was removed but not replaced with a shutdown() call

The old code at kEos performed full teardown: it moved out local_participant_, remote_participants_, room_handle_, e2ee_manager_, stream readers, called local_participant->shutdown(), and removed the FFI listener via removeListener(). The new code at src/room.cpp:1221-1226 only fires the delegate callback and does nothing else.

The new shutdown() method's own comment at include/livekit/room.h:359 says it is the "Shared shutdown path for explicit disconnect, server disconnect, EOS, and destruction," confirming the intent was for EOS to use it. But the kEos case never calls shutdown().

The kDisconnected handler at src/room.cpp:1195-1198 does call shutdown(), but if kEos arrives without a preceding kDisconnected (e.g., abnormal server termination, or a protocol edge case), the room handle, local participant, remote participants, E2EE manager, stream readers, and the FFI listener all leak.

(Refers to lines 1221-1226)

Was this helpful? React with 👍 or 👎 to provide feedback.

alan-george-lk added 4 commits July 13, 2026 10:17

Initial impl of shutdown repro

8f5433c

Use room name for repro

d5dda8c

Implementation of teardown feature for bugfix

0b9ea06

Add integration test

9effbcb

alan-george-lk changed the title ~~Bugfix: server-initiated room disconnect teardown fix~~ Bugfix: Server-initiated room disconnect teardown fix Jul 13, 2026

alan-george-lk and others added 3 commits July 13, 2026 12:35

Remove standalone disconnect repro from branch

738693b

Co-authored-by: Cursor <cursoragent@cursor.com>

Cleanup

0672226

Unify emitFfiEvent into standalone header

7810fa3

alan-george-lk marked this pull request as ready for review July 13, 2026 22:36

alan-george-lk requested review from stephen-derosa and xianshijing-lk as code owners July 13, 2026 22:36

devin-ai-integration Bot reviewed Jul 13, 2026

View reviewed changes

xianshijing-lk approved these changes Jul 14, 2026

View reviewed changes

alan-george-lk and others added 2 commits July 13, 2026 21:53

Merge branch 'main' of github.com:livekit/client-sdk-cpp into alan/bu…

9f56644

…gfix-room-shutdown

alan-george-lk changed the title ~~Bugfix: Server-initiated room disconnect teardown fix~~ Bugfix: Server-initiated room disconnect shutdown fix Jul 14, 2026

This comment was marked as resolved.

Sign in to view

Cleanup weird test

a2e6a9e

devin-ai-integration Bot reviewed Jul 14, 2026

View reviewed changes

stephen-derosa reviewed Jul 14, 2026

View reviewed changes

stephen-derosa approved these changes Jul 14, 2026

View reviewed changes

alan-george-lk force-pushed the alan/bugfix-room-shutdown branch from 3cbddb6 to a2e6a9e Compare July 14, 2026 19:12

alan-george-lk added 3 commits July 14, 2026 18:58

Merge branch 'main' of github.com:livekit/client-sdk-cpp into alan/bu…

e8569e3

…gfix-room-shutdown

Removing shutdown from EOS event

7534b22

Address unrecoverable case

55299c3

devin-ai-integration Bot reviewed Jul 15, 2026

View reviewed changes

Uh oh!

Conversation

alan-george-lk commented Jul 13, 2026 • edited by stephen-derosa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Ticket

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

xianshijing-lk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xianshijing-lk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xianshijing-lk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

stephen-derosa Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

xianshijing-lk commented Jul 14, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephen-derosa Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stephen-derosa Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

stephen-derosa left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alan-george-lk commented Jul 13, 2026 •

edited by stephen-derosa

Loading

alan-george-lk Jul 14, 2026 •

edited

Loading

devin-ai-integration Bot Jul 14, 2026 •

edited

Loading