Report client-initiated disconnects through Room.Disconnected

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. 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
 
 #### Publishing a texture (e.g Unity Camera)

Runtime/Scripts/Core/Room.cs

@@ -112,6 +112,9 @@ 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;
         private readonly Dictionary<string, RemoteParticipant> _participants = new();
         private StreamHandlerRegistry _streamHandlers = new();
@@ -190,20 +193,22 @@ public ConnectInstruction Connect(string url, string token, RoomOptions options)
             return instruction;
         }
 
+        /// <summary>
+        /// Disconnects from the room. A local disconnect is reported the way a
+        /// server-side one is — <see cref="ConnectionStateChanged"/>,
+        /// <see cref="Disconnected"/> and <see cref="DisconnectedWithReason"/> with
+        /// <see cref="DisconnectReason.ClientInitiated"/>, synchronously and with the
+        /// room's handles still live — so one teardown path covers both.
+        /// <see cref="Dispose"/> 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 <see cref="Connect"/> is still pending: that connect completes,
+        /// and the app has to disconnect again once it has.
+        /// </summary>
         public void Disconnect()
         {
             if (_disposed || RoomHandle == null)
                 return;
-            var (response, _) = FFIBridge.Instance.SendDisconnectRequest(this);
-            using (response)
-            {
-                Utils.Debug($"Disconnect.... {RoomHandle}");
-                Utils.Debug($"Disconnect response.... {response}");
-            }
-            // 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();
+            Teardown(DisconnectReason.ClientInitiated, closeFfiRoom: true);
         }
 
         public void Dispose()
@@ -212,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();
@@ -235,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);
+        }
+
         /// <summary>
         /// Registers a handler for incoming text streams matching the given topic.
         /// </summary>
@@ -308,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;
@@ -542,14 +607,17 @@ internal void OnEventReceived(RoomEvent e)
                     }
                     break;
                 case RoomEvent.MessageOneofCase.ConnectionStateChanged:
-                    ConnectionState = e.ConnectionStateChanged.State;
-                    ConnectionStateChanged?.Invoke(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:
-                    DisconnectReason = e.Disconnected.Reason;
-                    Disconnected?.Invoke(this);
-                    DisconnectedWithReason?.Invoke(this, DisconnectReason);
-                    OnDisconnect();
+                    Teardown(e.Disconnected.Reason, closeFfiRoom: false);
                     break;
                 case RoomEvent.MessageOneofCase.Reconnecting:
                     Reconnecting?.Invoke(this);
@@ -590,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);
@@ -604,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<ReadyForRoomEventRequest>())
@@ -613,6 +684,15 @@ internal void OnConnect(ConnectCallback info)
                 using var readyResponse = readyRequest.Send();
             }
 
+            // 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);
         }
 
@@ -628,15 +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.
-            DisconnectReason = DisconnectReason.UnknownReason;
-            Disconnected?.Invoke(this);
-            DisconnectedWithReason?.Invoke(this, DisconnectReason);
-            OnDisconnect();
-        }
-
-        private void OnDisconnect()
-        {
-            Cleanup();
+            Teardown(DisconnectReason.UnknownReason, closeFfiRoom: false);
         }
 
         internal RemoteParticipant CreateRemoteParticipantWithTracks(ConnectCallback.Types.ParticipantWithTracks item)
@@ -694,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()

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)
     {

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);
+        }
     }
 }