Skip to content

Room disconnect issue fix#146

Merged
alan-george-lk merged 7 commits into
mainfrom
feature/room_disconnect
Jun 1, 2026
Merged

Room disconnect issue fix#146
alan-george-lk merged 7 commits into
mainfrom
feature/room_disconnect

Conversation

@alan-george-lk
Copy link
Copy Markdown
Collaborator

@alan-george-lk alan-george-lk commented May 29, 2026

This PR does the following:

Testing

  • Added unit and integration tests verifying behavior and replicating original user-reported issue through a room.reset() smart pointer invoked destruction

Comment thread include/livekit/room.h Outdated
Comment thread src/tests/integration/test_room.cpp
Comment thread src/ffi_client.h Outdated
Comment thread src/local_participant.cpp Outdated
Comment thread src/room.cpp Outdated
Comment thread src/room.cpp Outdated
alan-george-lk and others added 4 commits May 29, 2026 13:53
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 <noreply@anthropic.com>
Comment thread src/ffi_client.cpp
logAndThrow("FfiResponse missing disconnect");
}
} catch (...) {
cancelPendingByAsyncId(async_id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LK_LOG_ERROR ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable but other operations in this file don't do that pattern in this catch, the above try block will log if that part goes wrong

Comment thread src/room.cpp
Copy link
Copy Markdown
Collaborator

@stephen-derosa stephen-derosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me -- synced with @ladvoc on this and the order of destroying all objects, then sending an async disconnect request is bang on

@alan-george-lk alan-george-lk marked this pull request as ready for review June 1, 2026 17:43
@alan-george-lk alan-george-lk merged commit c362267 into main Jun 1, 2026
53 of 56 checks passed
@alan-george-lk alan-george-lk deleted the feature/room_disconnect branch June 1, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Room destructor does not disconnect from server; shutdown() must be called first (undocumented requirement)

3 participants