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

* 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.

* squash: synchronize, refactor.

---------

Co-authored-by: Boris Grozev <[email protected]>

Author: damencho

jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java

@@ -640,17 +640,31 @@ private void leaveTheRoom()
             chatRoomRoleManager.stop();
         }
 
-        chatRoom.leave();
+        // first disconnect vnodes before leaving
+        final List<ExtensionElement> disconnectVnodeExtensions;
+        final List<ChatRoom> visitorChatRoomsToLeave;
+        synchronized (visitorChatRooms)
+        {
+            disconnectVnodeExtensions = visitorChatRooms.keySet().stream()
+                    .map(DisconnectVnodePacketExtension::new).collect(Collectors.toList());
+            visitorChatRoomsToLeave = new ArrayList<>(visitorChatRooms.values());
+            visitorChatRooms.clear();
+        }
 
+        final ChatRoom chatRoomToLeave = chatRoom;
         chatRoom.removeListener(chatRoomListener);
         chatRoom = null;
 
-        List<ExtensionElement> disconnectVnodeExtensions = new ArrayList<>();
-        synchronized (visitorChatRooms)
+        TaskPools.getIoPool().submit(() ->
         {
-            visitorChatRooms.forEach((vnode, visitorChatRoom) ->
+            if (!disconnectVnodeExtensions.isEmpty())
+            {
+                jicofoServices.getXmppServices().getVisitorsManager()
+                        .sendIqToComponentAndGetResponse(roomName, disconnectVnodeExtensions);
+            }
+
+            visitorChatRoomsToLeave.forEach(visitorChatRoom ->
             {
-                disconnectVnodeExtensions.add(new DisconnectVnodePacketExtension(vnode));
                 try
                 {
                     visitorChatRoom.removeAllListeners();
@@ -661,14 +675,9 @@ private void leaveTheRoom()
                     logger.error("Failed to leave visitor room", e);
                 }
             });
-            visitorChatRooms.clear();
-        }
 
-        if (!disconnectVnodeExtensions.isEmpty())
-        {
-            jicofoServices.getXmppServices().getVisitorsManager()
-                    .sendIqToComponent(roomName, disconnectVnodeExtensions);
-        }
+            chatRoomToLeave.leave();
+        });
     }
 
     /**
@@ -2372,6 +2381,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);
         }
@@ -2382,6 +2392,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);
         }

jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/VisitorsManager.kt

@@ -57,17 +57,24 @@ class VisitorsManager(
         logger.info("VisitorsComponentManager is now ${if (enabled) "en" else "dis"}abled with address $address")
     }
 
-    fun sendIqToComponent(roomJid: EntityBareJid, extensions: List<ExtensionElement>) {
+    private fun createIq(roomJid: EntityBareJid, extensions: List<ExtensionElement>): VisitorsIq {
         val address = this.address ?: throw Exception("Component not available.")
-        val iq = VisitorsIq.Builder(xmppProvider.xmppConnection).apply {
+        return VisitorsIq.Builder(xmppProvider.xmppConnection).apply {
             to(address)
             ofType(IQ.Type.get)
             room = roomJid
             addExtensions(extensions)
         }.build()
+    }
 
+    /** Send an IQ, block for response or timeout, return the result. */
+    fun sendIqToComponentAndGetResponse(roomJid: EntityBareJid, extensions: List<ExtensionElement>): IQ? =
+        xmppProvider.xmppConnection.sendIqAndGetResponse(createIq(roomJid, extensions))
+
+    /** Send an IQ, return immediately. Log an error if there's no response. */
+    fun sendIqToComponent(roomJid: EntityBareJid, extensions: List<ExtensionElement>) {
         TaskPools.ioPool.submit {
-            val response = xmppProvider.xmppConnection.sendIqAndGetResponse(iq)
+            val response = sendIqToComponentAndGetResponse(roomJid, extensions)
             when {
                 response == null -> logger.warn("Timeout waiting for VisitorsIq response.")
                 response.type == IQ.Type.result -> {