Skip to content

Commit c33a190

Browse files
Additional race fix
1 parent c243cf7 commit c33a190

2 files changed

Lines changed: 38 additions & 5 deletions

File tree

src/room.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ bool Room::shutdown(bool disconnect_ffi, DisconnectReason reason, bool notify_de
224224
std::unordered_map<std::string, std::shared_ptr<TextStreamReader>> text_stream_readers_to_clear;
225225
std::unordered_map<std::string, std::shared_ptr<ByteStreamReader>> byte_stream_readers_to_clear;
226226
int listener_to_remove = 0;
227+
bool claimed_disconnect = false;
227228

228229
{
229230
const std::scoped_lock<std::mutex> g(lock_);
@@ -234,6 +235,10 @@ bool Room::shutdown(bool disconnect_ffi, DisconnectReason reason, bool notify_de
234235
if (!has_room_state) {
235236
return false;
236237
}
238+
// The state transition determines which racing path owns the FFI request
239+
// and delegate notification. Remaining room state is still claimed here so
240+
// EOS or destruction can finish local cleanup after a server disconnect.
241+
claimed_disconnect = connection_state_ != ConnectionState::Disconnected;
237242
handle = std::move(room_handle_);
238243
delegate_snapshot = delegate_;
239244
local_participant_to_cleanup = std::move(local_participant_);
@@ -259,7 +264,7 @@ bool Room::shutdown(bool disconnect_ffi, DisconnectReason reason, bool notify_de
259264
}
260265
}
261266

262-
if (disconnect_ffi && handle && handle->valid()) {
267+
if (disconnect_ffi && claimed_disconnect && handle && handle->valid()) {
263268
try {
264269
FfiClient::instance().disconnectAsync(handle->get(), reason).get();
265270
} catch (const std::exception& e) {
@@ -302,7 +307,7 @@ bool Room::shutdown(bool disconnect_ffi, DisconnectReason reason, bool notify_de
302307
byte_stream_readers_to_clear.clear();
303308
handle.reset();
304309

305-
if (notify_delegate && delegate_snapshot) {
310+
if (notify_delegate && claimed_disconnect && delegate_snapshot) {
306311
DisconnectedEvent ev;
307312
ev.reason = reason;
308313
try {
@@ -314,7 +319,7 @@ bool Room::shutdown(bool disconnect_ffi, DisconnectReason reason, bool notify_de
314319
}
315320
}
316321

317-
return shutdown_ok;
322+
return claimed_disconnect && shutdown_ok;
318323
}
319324

320325
RoomInfoData Room::roomInfo() const {
@@ -1200,6 +1205,7 @@ void Room::onEvent(const FfiEvent& event) {
12001205
// and notifies the delegate itself. Suppress that duplicate while
12011206
// passing server-initiated disconnects through unchanged.
12021207
should_notify = connection_state_ != ConnectionState::Disconnected;
1208+
connection_state_ = ConnectionState::Disconnected;
12031209
}
12041210
if (should_notify && delegate_snapshot) {
12051211
DisconnectedEvent ev;

src/tests/unit/test_room.cpp

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ TEST_F(RoomTest, RemoteParticipantLookupBeforeConnect) {
333333
<< "Looking up participant before connect should return an empty handle";
334334
}
335335

336-
TEST_F(RoomTest, ServerDisconnectNotifiesBeforeEosTearsDownRoom) {
336+
TEST_F(RoomTest, ServerDisconnectMarksDisconnectedBeforeEosTearsDownRoom) {
337337
Room room;
338338
UnitLifecycleTrackingDelegate delegate;
339339
std::atomic<int> listener_calls{0};
@@ -348,7 +348,7 @@ TEST_F(RoomTest, ServerDisconnectNotifiesBeforeEosTearsDownRoom) {
348348
emitFfiEvent(disconnected_event);
349349

350350
EXPECT_EQ(listener_calls.load(std::memory_order_relaxed), 1);
351-
EXPECT_EQ(room.connectionState(), ConnectionState::Connected);
351+
EXPECT_EQ(room.connectionState(), ConnectionState::Disconnected);
352352
EXPECT_TRUE(RoomTestAccess::hasRoomHandle(room));
353353
EXPECT_NE(RoomTestAccess::listenerId(room), 0);
354354
ASSERT_EQ(delegate.callbacks.size(), 1);
@@ -374,4 +374,31 @@ TEST_F(RoomTest, ServerDisconnectNotifiesBeforeEosTearsDownRoom) {
374374
EXPECT_FALSE(room.disconnect()) << "disconnect after EOS teardown must be a no-op";
375375
}
376376

377+
TEST_F(RoomTest, DisconnectAfterServerDisconnectCleansUpWithoutDuplicateNotification) {
378+
Room room;
379+
UnitLifecycleTrackingDelegate delegate;
380+
std::atomic<int> listener_calls{0};
381+
room.setDelegate(&delegate);
382+
RoomTestAccess::installConnectedListener(room, listener_calls);
383+
384+
proto::FfiEvent disconnected_event;
385+
auto* disconnected_room_event = disconnected_event.mutable_room_event();
386+
disconnected_room_event->set_room_handle(0);
387+
disconnected_room_event->mutable_disconnected()->set_reason(proto::ROOM_DELETED);
388+
389+
emitFfiEvent(disconnected_event);
390+
391+
ASSERT_EQ(delegate.callbacks.size(), 1);
392+
EXPECT_EQ(delegate.callbacks[0], LifecycleCallback::Disconnected);
393+
EXPECT_EQ(delegate.reason, DisconnectReason::RoomDeleted);
394+
EXPECT_EQ(room.connectionState(), ConnectionState::Disconnected);
395+
EXPECT_TRUE(RoomTestAccess::hasRoomHandle(room));
396+
397+
EXPECT_FALSE(room.disconnect()) << "disconnect must not reclaim an already-disconnected transition";
398+
EXPECT_EQ(room.connectionState(), ConnectionState::Disconnected);
399+
EXPECT_FALSE(RoomTestAccess::hasRoomHandle(room));
400+
EXPECT_EQ(RoomTestAccess::listenerId(room), 0);
401+
EXPECT_EQ(delegate.callbacks.size(), 1) << "onDisconnected must not fire twice";
402+
}
403+
377404
} // namespace livekit::test

0 commit comments

Comments
 (0)