fix(visitors): Make sure we disconnect vnodes before leaving rooms.

Use case is a single main participant and a visitor node. If jicofo leaves visitor room before disconnecting, the client will reload seeing jicofo leaving. If we disconnect first, we destroy the room with an appropriate message shown to visitors.
jitsi · Sep 18, 2024 · f65f3d6 · f65f3d6
1 parent 1effdfd
commit f65f3d6
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 27 deletions.
diff --git a/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java b/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java
@@ -644,35 +644,41 @@ private void leaveTheRoom()
             chatRoomRoleManager.stop();
         }
 
-        chatRoom.leave();
-
-        chatRoom.removeListener(chatRoomListener);
-        chatRoom = null;
-
-        List<ExtensionElement> disconnectVnodeExtensions = new ArrayList<>();
-        synchronized (visitorChatRooms)
-        {
-            visitorChatRooms.forEach((vnode, visitorChatRoom) ->
-            {
-                disconnectVnodeExtensions.add(new DisconnectVnodePacketExtension(vnode));
-                try
-                {
-                    visitorChatRoom.removeAllListeners();
-                    visitorChatRoom.leave();
-                }
-                catch (Exception e)
-                {
-                    logger.error("Failed to leave visitor room", e);
-                }
-            });
-            visitorChatRooms.clear();
-        }
+        // first disconnect vnodes before leaving
+        List<ExtensionElement> disconnectVnodeExtensions = visitorChatRooms.keySet().stream()
+                .map(DisconnectVnodePacketExtension::new).collect(Collectors.toList());
 
         if (!disconnectVnodeExtensions.isEmpty())
         {
             jicofoServices.getXmppServices().getVisitorsManager()
-                    .sendIqToComponent(roomName, disconnectVnodeExtensions);
+                    .sendIqToComponent(roomName, disconnectVnodeExtensions, () -> {
+                        synchronized (visitorChatRooms)
+                        {
+                            visitorChatRooms.values().forEach(visitorChatRoom -> {
+                                try
+                                {
+                                    visitorChatRoom.removeAllListeners();
+                                    visitorChatRoom.leave();
+                                }
+                                catch (Exception e)
+                                {
+                                    logger.error("Failed to leave visitor room", e);
+                                }
+                            });
+                            visitorChatRooms.clear();
+                        }
+
+                        chatRoom.leave();
+                        return null;
+                    });
         }
+        else
+        {
+            chatRoom.leave();
+        }
+
+        chatRoom.removeListener(chatRoomListener);
+        chatRoom = null;
     }
 
     /**
@@ -1891,7 +1897,7 @@ private String selectVisitorNode()
             VisitorsManager visitorsManager = jicofoServices.getXmppServices().getVisitorsManager();
             visitorsManager.sendIqToComponent(
                     roomName,
-                    Collections.singletonList(new ConnectVnodePacketExtension(node)));
+                    Collections.singletonList(new ConnectVnodePacketExtension(node)), null);
         }
         else
         {
@@ -2318,7 +2324,7 @@ public void roomDestroyed(String reason)
                 if (vnode != null)
                 {
                     jicofoServices.getXmppServices().getVisitorsManager().sendIqToComponent(
-                            roomName, Collections.singletonList(new DisconnectVnodePacketExtension(vnode)));
+                            roomName, Collections.singletonList(new DisconnectVnodePacketExtension(vnode)), null);
                 }
             }
         }
@@ -2342,6 +2348,7 @@ public void memberKicked(@NotNull ChatRoomMember member)
             if (member.getRole() != MemberRole.VISITOR)
             {
                 logger.debug("Member kicked for non-visitor member of visitor room: " + member);
+                return;
             }
             onMemberKicked(member);
         }
@@ -2352,6 +2359,7 @@ public void memberLeft(@NotNull ChatRoomMember member)
             if (member.getRole() != MemberRole.VISITOR)
             {
                 logger.debug("Member left for non-visitor member of visitor room: " + member);
+                return;
             }
             onMemberLeft(member);
         }

diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/VisitorsManager.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/VisitorsManager.kt
@@ -57,7 +57,7 @@ class VisitorsManager(
         logger.info("VisitorsComponentManager is now ${if (enabled) "en" else "dis"}abled with address $address")
     }
 
-    fun sendIqToComponent(roomJid: EntityBareJid, extensions: List<ExtensionElement>) {
+    fun sendIqToComponent(roomJid: EntityBareJid, extensions: List<ExtensionElement>, callback: (() -> Unit)?) {
         val address = this.address ?: throw Exception("Component not available.")
         val iq = VisitorsIq.Builder(xmppProvider.xmppConnection).apply {
             to(address)
@@ -75,6 +75,8 @@ class VisitorsManager(
                 }
                 else -> logger.warn("Received error response: ${response.toStringOpt()}")
             }
+
+            callback?.invoke()
         }
     }