From 1dc3494f1526a924f8c1e1c26d8d45d49bcd8195 Mon Sep 17 00:00:00 2001 From: Max Heimbrock <43608204+MaxHeimbrock@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:31:40 +0200 Subject: [PATCH 1/3] Report client-initiated disconnects through Room.Disconnected A local Disconnect() or Dispose() now raises ConnectionStateChanged, Disconnected and DisconnectedWithReason with DisconnectReason.ClientInitiated, synchronously and before Cleanup releases the handles, as the other LiveKit SDKs and the Rust core do. Previously Cleanup unsubscribed the room from FFI events before the core's own Disconnected event could arrive, so apps needed two teardown paths. The report happens once per room across the remote event, panic and local paths, so a handler calling Disconnect() re-entrantly is a no-op. OnConnect records ConnConnected, which the core records before this wrapper subscribes to room events, so IsConnected is true on a connected room; repeats of the current state arriving from the core are dropped. Un-ignores the two RoomTests that covered these gaps and adds two more. Co-Authored-By: Claude Fable 5.1 --- README.md | 2 ++ Runtime/Scripts/Core/Room.cs | 61 ++++++++++++++++++++++++++++++------ Tests/PlayMode/RoomTests.cs | 55 ++++++++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 284dee4c..0eaea7ff 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,8 @@ IEnumerator ConnectToRoom() } ``` +Subscribe to `room.Disconnected` (or `room.DisconnectedWithReason`) for your teardown: it is raised for server-side disconnects and, with `DisconnectReason.ClientInitiated`, for your own `room.Disconnect()` or `Dispose()` as well, so one handler covers both. `room.ConnectionStateChanged` reports the same transitions, and `room.IsConnected` is true from the moment `Connected` is raised. + ### Video #### Publishing a texture (e.g Unity Camera) diff --git a/Runtime/Scripts/Core/Room.cs b/Runtime/Scripts/Core/Room.cs index 44d52302..aad55363 100644 --- a/Runtime/Scripts/Core/Room.cs +++ b/Runtime/Scripts/Core/Room.cs @@ -113,6 +113,11 @@ public class Room : IDisposable { internal FfiHandle RoomHandle = null; private bool _disposed = false; + // Set once the disconnect has been reported to the app (Disconnected / + // DisconnectedWithReason raised) by whichever path got there first: the remote + // Disconnected event, a panic, or a local Disconnect(). Keeps the report to + // exactly one per room, also when a handler calls Disconnect() re-entrantly. + private bool _disconnectReported; private readonly Dictionary _participants = new(); private StreamHandlerRegistry _streamHandlers = new(); @@ -190,9 +195,19 @@ public ConnectInstruction Connect(string url, string token, RoomOptions options) return instruction; } + /// + /// Disconnects from the room. Like the other LiveKit SDKs, a local disconnect is + /// reported through , + /// and with + /// — synchronously, before the + /// room's resources are released — so one teardown path covers hang-ups and + /// server-side disconnects alike. goes through here too. + /// Calling this from a Disconnected handler, or on a room that already reported + /// its disconnect, is a no-op. + /// public void Disconnect() { - if (_disposed || RoomHandle == null) + if (_disposed || _disconnectReported || RoomHandle == null) return; var (response, _) = FFIBridge.Instance.SendDisconnectRequest(this); using (response) @@ -200,12 +215,40 @@ public void Disconnect() Utils.Debug($"Disconnect.... {RoomHandle}"); Utils.Debug($"Disconnect response.... {response}"); } + // The Rust core reports its own close as a ClientInitiated Disconnected event, + // but Cleanup below unsubscribes this room from FFI events before that event + // could arrive, so the wrapper reports it itself — ahead of Cleanup, so + // handlers still find the room intact, as they do on a server-side disconnect. + ReportDisconnected(DisconnectReason.ClientInitiated); // Release the Rust-side room synchronously. Without this the FfiRoom // (peer connection, signaling client, libwebrtc state) lingers in the // FFI handle table until the SafeHandle finalizer runs. Cleanup(); } + private void ReportDisconnected(DisconnectReason reason) + { + if (_disconnectReported) + return; + _disconnectReported = true; + DisconnectReason = reason; + SetConnectionState(ConnectionState.ConnDisconnected); + Disconnected?.Invoke(this); + DisconnectedWithReason?.Invoke(this, reason); + } + + // Records a connection-state transition and raises ConnectionStateChanged for it. + // A repeat of the current state is dropped, so a transition this wrapper records + // itself (Connected in OnConnect, Disconnected in ReportDisconnected) and the same + // transition arriving from the Rust core are reported once. + private void SetConnectionState(ConnectionState state) + { + if (ConnectionState == state) + return; + ConnectionState = state; + ConnectionStateChanged?.Invoke(state); + } + public void Dispose() { Disconnect(); @@ -542,13 +585,10 @@ internal void OnEventReceived(RoomEvent e) } break; case RoomEvent.MessageOneofCase.ConnectionStateChanged: - ConnectionState = e.ConnectionStateChanged.State; - ConnectionStateChanged?.Invoke(e.ConnectionStateChanged.State); + SetConnectionState(e.ConnectionStateChanged.State); break; case RoomEvent.MessageOneofCase.Disconnected: - DisconnectReason = e.Disconnected.Reason; - Disconnected?.Invoke(this); - DisconnectedWithReason?.Invoke(this, DisconnectReason); + ReportDisconnected(e.Disconnected.Reason); OnDisconnect(); break; case RoomEvent.MessageOneofCase.Reconnecting: @@ -613,6 +653,11 @@ internal void OnConnect(ConnectCallback info) using var readyResponse = readyRequest.Send(); } + // The Rust core recorded this transition during connect, before this room was + // subscribed to its events; record it here so IsConnected is true from the + // moment Connected is raised. The core's own copy of the event, if it still + // arrives, is dropped as a repeat. + SetConnectionState(ConnectionState.ConnConnected); Connected?.Invoke(this); } @@ -628,9 +673,7 @@ private void OnPanicReceived(Panic e) // room could silently stop receiving events (including Disconnected // itself), so the panic is surfaced through the disconnect path apps // already handle. - DisconnectReason = DisconnectReason.UnknownReason; - Disconnected?.Invoke(this); - DisconnectedWithReason?.Invoke(this, DisconnectReason); + ReportDisconnected(DisconnectReason.UnknownReason); OnDisconnect(); } diff --git a/Tests/PlayMode/RoomTests.cs b/Tests/PlayMode/RoomTests.cs index 669b0dc6..90fa5f13 100644 --- a/Tests/PlayMode/RoomTests.cs +++ b/Tests/PlayMode/RoomTests.cs @@ -129,7 +129,7 @@ public IEnumerator RoomSid_StartsWithRM() StringAssert.StartsWith("RM_", context.Rooms[0].Sid); } - [UnityTest, Category("E2E"), Ignore("Known issue")] + [UnityTest, Category("E2E")] public IEnumerator ConnectionState_IsConnected() { using var context = new TestRoomContext(); @@ -232,7 +232,7 @@ public IEnumerator ParticipantDisconnect_TriggersEvent() if (expectation.Error != null) Assert.Fail(expectation.Error); } - [UnityTest, Category("E2E"), Ignore("Known issue")] + [UnityTest, Category("E2E")] public IEnumerator Disconnect_TriggersEvent() { using var context = new TestRoomContext(); @@ -267,5 +267,56 @@ public IEnumerator Disconnect_TriggersEvent() yield return expectation.Wait(); if (expectation.Error != null) Assert.Fail(expectation.Error); } + + [UnityTest, Category("E2E")] + public IEnumerator Disconnect_ReportsClientInitiated_Once() + { + using var context = new TestRoomContext(); + yield return context.ConnectAll(); + Assert.IsNull(context.ConnectionError, context.ConnectionError); + var room = context.Rooms[0]; + + var disconnected = 0; + DisconnectReason? reason = null; + room.Disconnected += _ => disconnected++; + room.DisconnectedWithReason += (_, r) => reason = r; + + room.Disconnect(); + + // Reported synchronously: no frame has to pass for a hang-up to be observable. + Assert.AreEqual(1, disconnected); + Assert.AreEqual(DisconnectReason.ClientInitiated, reason); + Assert.AreEqual(DisconnectReason.ClientInitiated, room.DisconnectReason); + Assert.AreEqual(ConnectionState.ConnDisconnected, room.ConnectionState); + Assert.IsFalse(room.IsConnected); + + // A second Disconnect (TestRoomContext.Dispose issues one too) reports nothing. + room.Disconnect(); + Assert.AreEqual(1, disconnected); + } + + [UnityTest, Category("E2E")] + public IEnumerator Disconnect_FromDisconnectedHandler_ReportsOnce() + { + using var context = new TestRoomContext(); + yield return context.ConnectAll(); + Assert.IsNull(context.ConnectionError, context.ConnectionError); + var room = context.Rooms[0]; + + var disconnected = 0; + room.Disconnected += r => + { + disconnected++; + // A teardown handler that hangs up "to be sure" must not re-report. + r.Disconnect(); + }; + + // Dispose is a disconnect too. + room.Dispose(); + + Assert.AreEqual(1, disconnected); + Assert.IsFalse(room.IsConnected); + } + } } \ No newline at end of file From a1e16e6b55b85beafc55cfdca1c88d10ebf4918c Mon Sep 17 00:00:00 2001 From: Max Heimbrock <43608204+MaxHeimbrock@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:21:37 +0200 Subject: [PATCH 2/3] Harden the disconnect path after self-review All three disconnect paths (local Disconnect/Dispose, the core's Disconnected event, panic) now exit through one Teardown(reason, closeFfiRoom): it marks the room disposed, unsubscribes from FFI events, raises the events with the handles still live, and releases the handles in a finally, so a throwing handler cannot leave the Rust-side room alive. The local path sends the FFI disconnect request after the handlers, because the Rust-side close replaces remote-track handles on its worker thread immediately. The core's ConnectionStateChanged(Disconnected) is recorded from the Disconnected{reason} event that always follows it, so handlers of either event see the same DisconnectReason on every path and a handler that disconnects in between cannot replace the server's reason with ClientInitiated. OnConnect resets the disposed flag so a Room can be connected again, and skips Connected when a ConnectionStateChanged handler already disconnected. ConnectInstruction completes in a finally so a throwing handler cannot hang the awaiting code. Comments now describe the mechanism as measured (the core's Connected copy always drains one pass later; the ready timeout is 15s). Tests pin the handles being live inside the handler, one report per event, the repeat-drop, a throwing handler, and a synthetic two-event server disconnect with re-entrant handlers. The pending-connect Disconnect() no-op stays and is documented. Co-Authored-By: Claude Fable 5.1 --- README.md | 2 +- Runtime/Scripts/Core/Room.cs | 197 ++++++++++++-------- Tests/EditMode/RoomDisconnectReasonTests.cs | 49 +++++ Tests/PlayMode/RoomTests.cs | 72 ++++--- 4 files changed, 218 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index 0eaea7ff..ed503eec 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ IEnumerator ConnectToRoom() } ``` -Subscribe to `room.Disconnected` (or `room.DisconnectedWithReason`) for your teardown: it is raised for server-side disconnects and, with `DisconnectReason.ClientInitiated`, for your own `room.Disconnect()` or `Dispose()` as well, so one handler covers both. `room.ConnectionStateChanged` reports the same transitions, and `room.IsConnected` is true from the moment `Connected` is raised. +Subscribe to `room.Disconnected` (or `room.DisconnectedWithReason`) for your teardown: it is raised for server-side disconnects and, with `DisconnectReason.ClientInitiated`, for your own `room.Disconnect()` or `Dispose()` as well, so one handler covers both. Handlers run before the room's handles are released. `room.ConnectionStateChanged` reports the same transitions, and `room.IsConnected` is true from the moment `Connected` is raised. One gap to know: a `Disconnect()` while the connect is still pending is a no-op, so disconnect again once `Connect` has completed. ### Video diff --git a/Runtime/Scripts/Core/Room.cs b/Runtime/Scripts/Core/Room.cs index aad55363..e74f80e0 100644 --- a/Runtime/Scripts/Core/Room.cs +++ b/Runtime/Scripts/Core/Room.cs @@ -112,12 +112,10 @@ public Proto.RoomOptions ToProto() public class Room : IDisposable { internal FfiHandle RoomHandle = null; + // Set at the start of Teardown, before the disconnect is reported, so a handler + // that calls Disconnect() re-entrantly is a no-op; reset by OnConnect so a Room + // can be connected again after a disconnect. private bool _disposed = false; - // Set once the disconnect has been reported to the app (Disconnected / - // DisconnectedWithReason raised) by whichever path got there first: the remote - // Disconnected event, a panic, or a local Disconnect(). Keeps the report to - // exactly one per room, also when a handler calls Disconnect() re-entrantly. - private bool _disconnectReported; private readonly Dictionary _participants = new(); private StreamHandlerRegistry _streamHandlers = new(); @@ -196,57 +194,21 @@ public ConnectInstruction Connect(string url, string token, RoomOptions options) } /// - /// Disconnects from the room. Like the other LiveKit SDKs, a local disconnect is - /// reported through , - /// and with - /// — synchronously, before the - /// room's resources are released — so one teardown path covers hang-ups and - /// server-side disconnects alike. goes through here too. - /// Calling this from a Disconnected handler, or on a room that already reported - /// its disconnect, is a no-op. + /// Disconnects from the room. A local disconnect is reported the way a + /// server-side one is — , + /// and with + /// , synchronously and with the + /// room's handles still live — so one teardown path covers both. + /// goes through here too. Calling this from a handler of + /// those events, or on a room that already disconnected, is a no-op. So is a + /// call while is still pending: that connect completes, + /// and the app has to disconnect again once it has. /// public void Disconnect() { - if (_disposed || _disconnectReported || RoomHandle == null) + if (_disposed || RoomHandle == null) return; - var (response, _) = FFIBridge.Instance.SendDisconnectRequest(this); - using (response) - { - Utils.Debug($"Disconnect.... {RoomHandle}"); - Utils.Debug($"Disconnect response.... {response}"); - } - // The Rust core reports its own close as a ClientInitiated Disconnected event, - // but Cleanup below unsubscribes this room from FFI events before that event - // could arrive, so the wrapper reports it itself — ahead of Cleanup, so - // handlers still find the room intact, as they do on a server-side disconnect. - ReportDisconnected(DisconnectReason.ClientInitiated); - // Release the Rust-side room synchronously. Without this the FfiRoom - // (peer connection, signaling client, libwebrtc state) lingers in the - // FFI handle table until the SafeHandle finalizer runs. - Cleanup(); - } - - private void ReportDisconnected(DisconnectReason reason) - { - if (_disconnectReported) - return; - _disconnectReported = true; - DisconnectReason = reason; - SetConnectionState(ConnectionState.ConnDisconnected); - Disconnected?.Invoke(this); - DisconnectedWithReason?.Invoke(this, reason); - } - - // Records a connection-state transition and raises ConnectionStateChanged for it. - // A repeat of the current state is dropped, so a transition this wrapper records - // itself (Connected in OnConnect, Disconnected in ReportDisconnected) and the same - // transition arriving from the Rust core are reported once. - private void SetConnectionState(ConnectionState state) - { - if (ConnectionState == state) - return; - ConnectionState = state; - ConnectionStateChanged?.Invoke(state); + Teardown(DisconnectReason.ClientInitiated, closeFfiRoom: true); } public void Dispose() @@ -255,20 +217,67 @@ public void Dispose() GC.SuppressFinalize(this); } - private void Cleanup() + // The single exit for every disconnect path — a local Disconnect(), the core's + // Disconnected event, a panic. Reports the disconnect once, then releases the + // room: handlers run first, with the handles still live, as they do on a + // server-side disconnect, and the release runs in a finally so a throwing + // handler cannot leave the Rust-side room alive. closeFfiRoom asks the Rust side + // to close the room (local path); on the other paths the core already has. + private void Teardown(DisconnectReason reason, bool closeFfiRoom) { if (_disposed) return; _disposed = true; + // Unsubscribed first: nothing still queued for this room — including the + // core's own Disconnected event for a local close — is processed from here on. FfiClient.Instance.RoomEventReceived -= OnEventReceived; FfiClient.Instance.RpcMethodInvocationReceived -= OnRpcMethodInvocationReceived; FfiClient.Instance.DisconnectReceived -= OnDisconnectReceived; FfiClient.Instance.PanicReceived -= OnPanicReceived; - // Participant + track + publication FFI handles are independent entries in the - // Rust handle table — dropping the room handle alone does not cascade to them, so - // they would otherwise linger until C# GC finalizes each SafeHandle. + try + { + DisconnectReason = reason; + SetConnectionState(ConnectionState.ConnDisconnected); + Disconnected?.Invoke(this); + DisconnectedWithReason?.Invoke(this, reason); + } + finally + { + try + { + if (closeFfiRoom) + CloseFfiRoom(); + } + finally + { + ReleaseHandles(); + } + } + } + + // Sent after the handlers on purpose: the Rust-side close replaces the remote + // track handles on its worker thread right away, so a handler touching tracks + // would otherwise race it. + private void CloseFfiRoom() + { + var (response, _) = FFIBridge.Instance.SendDisconnectRequest(this); + using (response) + { + Utils.Debug($"Disconnect.... {RoomHandle}"); + Utils.Debug($"Disconnect response.... {response}"); + } + } + + // Released synchronously. Without this the FfiRoom (peer connection, signaling + // client, libwebrtc state) lingers in the FFI handle table until the SafeHandle + // finalizer runs. Participant + track + publication FFI handles are independent + // entries in the Rust handle table — dropping the room handle alone does not + // cascade to them, so they would otherwise linger until C# GC finalizes each + // SafeHandle. + private void ReleaseHandles() + { LocalParticipant?.DisposeHandles(); foreach (var p in _participants.Values) p.DisposeHandles(); @@ -278,6 +287,19 @@ private void Cleanup() RoomHandle = null; } + // Records a connection-state transition and raises ConnectionStateChanged for it. + // A repeat of the current state is dropped: the core's own ConnectionStateChanged + // (Connected) is queued behind the connect callback and drains one event pass + // after OnConnect recorded the transition, so this check is what keeps every + // connect to a single Connected report. + private void SetConnectionState(ConnectionState state) + { + if (ConnectionState == state) + return; + ConnectionState = state; + ConnectionStateChanged?.Invoke(state); + } + /// /// Registers a handler for incoming text streams matching the given topic. /// @@ -351,7 +373,7 @@ internal void OnRpcMethodInvocationReceived(RpcMethodInvocationEvent e) internal void OnEventReceived(RoomEvent e) { - // After Cleanup() the handle is null but late events may still flow + // After Teardown the handle is null but late events may still flow // through the FfiClient before the unsubscribe fully takes effect. if (RoomHandle == null) return; @@ -585,11 +607,17 @@ internal void OnEventReceived(RoomEvent e) } break; case RoomEvent.MessageOneofCase.ConnectionStateChanged: - SetConnectionState(e.ConnectionStateChanged.State); + // The core reports a disconnect as ConnectionStateChanged(Disconnected) + // followed by Disconnected{reason}. The transition is recorded from + // the latter, once the reason is known, so handlers of either event + // see the same DisconnectReason on every path, and a handler that + // disconnects in between cannot replace the core's reason with + // ClientInitiated. + if (e.ConnectionStateChanged.State != ConnectionState.ConnDisconnected) + SetConnectionState(e.ConnectionStateChanged.State); break; case RoomEvent.MessageOneofCase.Disconnected: - ReportDisconnected(e.Disconnected.Reason); - OnDisconnect(); + Teardown(e.Disconnected.Reason, closeFfiRoom: false); break; case RoomEvent.MessageOneofCase.Reconnecting: Reconnecting?.Invoke(this); @@ -630,6 +658,9 @@ internal void OnEventReceived(RoomEvent e) internal void OnConnect(ConnectCallback info) { RoomHandle = FfiHandle.FromOwnedHandle(info.Result.Room.Handle); + // A Room may be connected again after a disconnect: each connect starts a + // fresh lifecycle. + _disposed = false; UpdateFromInfo(info.Result.Room.Info); LocalParticipant = new LocalParticipant(info.Result.LocalParticipant, this); @@ -644,7 +675,7 @@ internal void OnConnect(ConnectCallback info) FfiClient.Instance.RpcMethodInvocationReceived += OnRpcMethodInvocationReceived; // Signal Rust that listeners are installed and it can start forwarding room events. - // Without this the FFI side parks for 1s after ConnectCallback and then drops the room + // Without this the FFI side parks for 15s after ConnectCallback and then drops the room // with ConnectionTimeout. Must run after the FfiClient.RoomEventReceived subscription // above so no event can race ahead of OnEventReceived. using (var readyRequest = FFIBridge.Instance.NewRequest()) @@ -653,11 +684,15 @@ internal void OnConnect(ConnectCallback info) using var readyResponse = readyRequest.Send(); } - // The Rust core recorded this transition during connect, before this room was - // subscribed to its events; record it here so IsConnected is true from the - // moment Connected is raised. The core's own copy of the event, if it still - // arrives, is dropped as a repeat. + // The core recorded this transition during connect, before this room was + // subscribed to its events (its own copy drains later and is dropped as a + // repeat, see SetConnectionState); recording it here makes IsConnected true + // from the moment Connected is raised. SetConnectionState(ConnectionState.ConnConnected); + // A ConnectionStateChanged handler may already have disconnected the room; + // Connected is not raised on a torn-down room. + if (_disposed) + return; Connected?.Invoke(this); } @@ -673,13 +708,7 @@ private void OnPanicReceived(Panic e) // room could silently stop receiving events (including Disconnected // itself), so the panic is surfaced through the disconnect path apps // already handle. - ReportDisconnected(DisconnectReason.UnknownReason); - OnDisconnect(); - } - - private void OnDisconnect() - { - Cleanup(); + Teardown(DisconnectReason.UnknownReason, closeFfiRoom: false); } internal RemoteParticipant CreateRemoteParticipantWithTracks(ConnectCallback.Types.ParticipantWithTracks item) @@ -737,18 +766,26 @@ void OnConnect(ConnectCallback e) return; bool success = string.IsNullOrEmpty(e.Error); - if (success) + try { - if (_roomOptions.E2EE != null) + if (success) { - _room.E2EEManager = new E2EEManager(_room.RoomHandle, _roomOptions.E2EE); - } + if (_roomOptions.E2EE != null) + { + _room.E2EEManager = new E2EEManager(_room.RoomHandle, _roomOptions.E2EE); + } - _room.OnConnect(e); + _room.OnConnect(e); + } + } + finally + { + // Completed even when a ConnectionStateChanged or Connected handler throws + // inside OnConnect, so the code awaiting the connect resumes instead of + // hanging forever. + IsError = !success; + IsDone = true; } - - IsError = !success; - IsDone = true; } void OnCanceled() diff --git a/Tests/EditMode/RoomDisconnectReasonTests.cs b/Tests/EditMode/RoomDisconnectReasonTests.cs index 11ed1ebc..046eed53 100644 --- a/Tests/EditMode/RoomDisconnectReasonTests.cs +++ b/Tests/EditMode/RoomDisconnectReasonTests.cs @@ -84,5 +84,54 @@ public void ParticipantDisconnected_SurfacesReasonOnEvent() Assert.IsNotNull(eventParticipant); Assert.AreEqual(identity, eventParticipant.Identity); } + + [Test] + public void Disconnected_ReentrantDisconnectFromHandlers_KeepsServerReason() + { + var room = new Room(); + room.RoomHandle = new FfiHandle(IntPtr.Zero); + // A fresh Room already reads ConnDisconnected (the enum default), so record a + // connected state first, as the core's own event would after a connect. + room.OnEventReceived(new RoomEvent + { + RoomHandle = 0, + ConnectionStateChanged = new ConnectionStateChanged { State = ConnectionState.ConnConnected } + }); + Assert.AreEqual(ConnectionState.ConnConnected, room.ConnectionState); + + var reports = 0; + DisconnectReason? reasonSeenByStateHandler = null; + room.ConnectionStateChanged += state => + { + if (state != ConnectionState.ConnDisconnected) return; + reasonSeenByStateHandler = room.DisconnectReason; + // An app that disconnects "to be sure" from its state handler. + room.Disconnect(); + }; + room.DisconnectedWithReason += (_, __) => + { + reports++; + room.Disconnect(); + }; + + // The core reports a server-side disconnect as two queued events. + room.OnEventReceived(new RoomEvent + { + RoomHandle = 0, + ConnectionStateChanged = new ConnectionStateChanged { State = ConnectionState.ConnDisconnected } + }); + room.OnEventReceived(new RoomEvent + { + RoomHandle = 0, + Disconnected = new Disconnected { Reason = DisconnectReason.ServerShutdown } + }); + + Assert.AreEqual(1, reports, "the disconnect is reported once"); + Assert.AreEqual(DisconnectReason.ServerShutdown, room.DisconnectReason, + "a re-entrant Disconnect() must not replace the server's reason with ClientInitiated"); + Assert.AreEqual(DisconnectReason.ServerShutdown, reasonSeenByStateHandler, + "the ConnectionStateChanged handler sees the reason already recorded"); + Assert.AreEqual(ConnectionState.ConnDisconnected, room.ConnectionState); + } } } diff --git a/Tests/PlayMode/RoomTests.cs b/Tests/PlayMode/RoomTests.cs index 90fa5f13..5cafba35 100644 --- a/Tests/PlayMode/RoomTests.cs +++ b/Tests/PlayMode/RoomTests.cs @@ -269,7 +269,28 @@ public IEnumerator Disconnect_TriggersEvent() } [UnityTest, Category("E2E")] - public IEnumerator Disconnect_ReportsClientInitiated_Once() + public IEnumerator Connect_ReportsConnectedOnce() + { + using var context = new TestRoomContext(); + var room = context.Rooms[0]; + var connectedReports = 0; + room.ConnectionStateChanged += s => + { + if (s == ConnectionState.ConnConnected) connectedReports++; + }; + + yield return context.ConnectAll(); + Assert.IsNull(context.ConnectionError, context.ConnectionError); + Assert.AreEqual(1, connectedReports, "recorded from the connect callback"); + + // The core's own ConnectionStateChanged(Connected) is queued behind the connect + // callback and drains on a later frame; it must be absorbed as a repeat. + for (var i = 0; i < 5; i++) yield return null; + Assert.AreEqual(1, connectedReports, "the core's copy of the transition must not be reported again"); + } + + [UnityTest, Category("E2E")] + public IEnumerator Disconnect_ReportsClientInitiated_OnceWithRoomIntact() { using var context = new TestRoomContext(); yield return context.ConnectAll(); @@ -277,46 +298,55 @@ public IEnumerator Disconnect_ReportsClientInitiated_Once() var room = context.Rooms[0]; var disconnected = 0; + var disconnectedWithReason = 0; DisconnectReason? reason = null; - room.Disconnected += _ => disconnected++; - room.DisconnectedWithReason += (_, r) => reason = r; + room.Disconnected += r => + { + disconnected++; + // Handlers run before the release, with the state already recorded. + Assert.IsNotNull(r.RoomHandle, "the room handle must still be live in the handler"); + Assert.IsFalse(r.LocalParticipant.Handle.IsClosed, "participant handles must still be live in the handler"); + Assert.AreEqual(ConnectionState.ConnDisconnected, r.ConnectionState); + Assert.AreEqual(DisconnectReason.ClientInitiated, r.DisconnectReason); + // A teardown handler that hangs up "to be sure" must not re-report. + r.Disconnect(); + }; + room.DisconnectedWithReason += (_, r) => + { + disconnectedWithReason++; + reason = r; + }; - room.Disconnect(); + // Dispose is a disconnect too. Reported synchronously: no frame has to pass + // for a hang-up to be observable. + room.Dispose(); - // Reported synchronously: no frame has to pass for a hang-up to be observable. Assert.AreEqual(1, disconnected); + Assert.AreEqual(1, disconnectedWithReason); Assert.AreEqual(DisconnectReason.ClientInitiated, reason); - Assert.AreEqual(DisconnectReason.ClientInitiated, room.DisconnectReason); Assert.AreEqual(ConnectionState.ConnDisconnected, room.ConnectionState); - Assert.IsFalse(room.IsConnected); + Assert.IsNull(room.RoomHandle, "the room is released once the handlers ran"); - // A second Disconnect (TestRoomContext.Dispose issues one too) reports nothing. + // A later Disconnect (TestRoomContext.Dispose issues one too) reports nothing. room.Disconnect(); Assert.AreEqual(1, disconnected); } [UnityTest, Category("E2E")] - public IEnumerator Disconnect_FromDisconnectedHandler_ReportsOnce() + public IEnumerator Disconnect_ThrowingHandler_StillReleasesRoom() { using var context = new TestRoomContext(); yield return context.ConnectAll(); Assert.IsNull(context.ConnectionError, context.ConnectionError); var room = context.Rooms[0]; - var disconnected = 0; - room.Disconnected += r => - { - disconnected++; - // A teardown handler that hangs up "to be sure" must not re-report. - r.Disconnect(); - }; + room.Disconnected += _ => throw new System.InvalidOperationException("handler failed"); - // Dispose is a disconnect too. - room.Dispose(); + Assert.Throws(() => room.Disconnect()); - Assert.AreEqual(1, disconnected); - Assert.IsFalse(room.IsConnected); + // The handler's exception surfaces, but the room is still released. + Assert.IsNull(room.RoomHandle, "the release must run even when a Disconnected handler throws"); + Assert.DoesNotThrow(() => room.Dispose()); } - } } \ No newline at end of file From 6bd52ecfed89e71800a93c1d7082c90a7a84f7c4 Mon Sep 17 00:00:00 2001 From: Max Heimbrock <43608204+MaxHeimbrock@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:03:38 +0200 Subject: [PATCH 3/3] Tear the Meet call down from OnDisconnected only With Room.Disconnect() reporting ClientInitiated through Disconnected, the sample no longer needs its own teardown behind the hang-up button: OnEndCall just disconnects, and OnDisconnected owns the teardown for the button, a server-side disconnect and OnDestroy alike. That also fixes the sample leaving tracks and the "connected" UI behind on a server-side disconnect, which it only logged before. OnDestroy loses its dead second Disconnect() call. Co-Authored-By: Claude Fable 5.1 --- Samples~/Meet/Assets/Runtime/MeetManager.cs | 35 ++++++++++++--------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/Samples~/Meet/Assets/Runtime/MeetManager.cs b/Samples~/Meet/Assets/Runtime/MeetManager.cs index 1dc27c71..ee8e91ac 100644 --- a/Samples~/Meet/Assets/Runtime/MeetManager.cs +++ b/Samples~/Meet/Assets/Runtime/MeetManager.cs @@ -142,17 +142,14 @@ private void Update() private void OnDestroy() { // Without this, scene change / app quit while connected leaves all tracks, - // streams, and their backing GPU/native resources allocated. - if (_room != null) - { - _room.Disconnect(); - _room = null; - } + // streams, and their backing GPU/native resources allocated. Disconnect reports + // back through OnDisconnected, which tears the call down; CleanUpAllTracks after + // it covers anything created without a room. + _room?.Disconnect(); CleanUpAllTracks(); _webCamTexture?.Stop(); _platformAudioSource?.Dispose(); _platformAudio?.Dispose(); - _room?.Disconnect(); } #endregion @@ -166,13 +163,9 @@ private void OnStartCall() private void OnEndCall() { - if (_room == null) return; - - _room.Disconnect(); - CleanUpAllTracks(); - _room = null; - _localId = null; - buttonBar.SetConnected(false); + // The SDK reports a local disconnect through OnDisconnected (with + // DisconnectReason.ClientInitiated), which owns the teardown. + _room?.Disconnect(); } private void OnToggleCamera() @@ -432,8 +425,20 @@ private void OnParticipantDisconnected(Participant participant, DisconnectReason DestroyParticipantTile(participant.Identity); } + // The one teardown path: raised for the hang-up button, a server-side disconnect + // (kick, room deleted, token expiry) and the scene going away (OnDestroy) alike, + // synchronously and with the room's handles still live. private void OnDisconnected(Room room) - => Debug.Log($"Disconnected from room: {room.DisconnectReason}"); + { + Debug.Log($"Disconnected from room: {room.DisconnectReason}"); + + CleanUpAllTracks(); + _room = null; + _localId = null; + // Destroyed already when this runs from OnDestroy during a scene unload. + if (buttonBar != null) + buttonBar.SetConnected(false); + } private void OnTrackMuted(TrackPublication publication, Participant participant) {