From f326b319444d0de48b28abb997969ec7afed80b9 Mon Sep 17 00:00:00 2001 From: bgrozev Date: Mon, 1 Aug 2022 16:51:27 -0500 Subject: [PATCH] fix: Clear old state when MUC is re-joined. (#952) * fix: Clear old state when MUC is re-joined. * fix: Fix syncronization issues (spotbugs). --- .../impl/protocol/xmpp/ChatRoomImpl.java | 48 +++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jitsi/impl/protocol/xmpp/ChatRoomImpl.java b/src/main/java/org/jitsi/impl/protocol/xmpp/ChatRoomImpl.java index 505305804d..2a438c2f58 100644 --- a/src/main/java/org/jitsi/impl/protocol/xmpp/ChatRoomImpl.java +++ b/src/main/java/org/jitsi/impl/protocol/xmpp/ChatRoomImpl.java @@ -18,6 +18,8 @@ package org.jitsi.impl.protocol.xmpp; import javax.xml.namespace.*; + +import edu.umd.cs.findbugs.annotations.*; import kotlin.*; import org.jetbrains.annotations.*; import org.jitsi.jicofo.*; @@ -40,6 +42,7 @@ import org.jxmpp.stringprep.*; import java.util.*; +import java.util.concurrent.*; import java.util.function.*; /** @@ -47,6 +50,10 @@ * * @author Pawel Domas */ +@SuppressFBWarnings( + value = "JLM_JSR166_UTILCONCURRENT_MONITORENTER", + justification = "We intentionally synchronize on [members] (a ConcurrentHashMap)." +) public class ChatRoomImpl implements ChatRoom, PresenceListener { @@ -96,7 +103,7 @@ public class ChatRoomImpl */ private final Consumer leaveCallback; - private final Map members = new HashMap<>(); + private final Map members = new ConcurrentHashMap<>(); /** * Local user role. @@ -212,6 +219,7 @@ public void join() throws SmackException, XMPPException, InterruptedException { // TODO: clean-up the way we figure out what nickname to use. + resetState(); joinAs(xmppProvider.getConfig().getUsername()); } @@ -225,6 +233,34 @@ public String getMainRoom() { return mainRoom; } + /** + * Prepare this {@link ChatRoomImpl} for a call to {@link #joinAs(Resourcepart)}, which send initial presence to + * the MUC. Resets any state that might have been set the previous time the MUC was joined. + */ + private void resetState() + { + synchronized (members) + { + if (!members.isEmpty()) + { + logger.warn("Removing " + members.size() + " stale members."); + members.clear(); + } + } + + synchronized (this) + { + role = null; + lastPresenceSent = null; + meetingId = null; + logger.addContext("meeting_id", ""); + isBreakoutRoom = false; + mainRoom = null; + avModerationEnabled.clear(); + whitelists.clear(); + } + } + private void joinAs(Resourcepart nickname) throws SmackException, XMPPException, InterruptedException { this.myOccupantJid = JidCreate.entityFullFrom(roomJid, nickname); @@ -236,7 +272,7 @@ private void joinAs(Resourcepart nickname) throws SmackException, XMPPException, // it indicates that the client lost its synchronization and causes // the MUC service to re-send the presence of each occupant in the // room. - synchronized (this) + synchronized (ChatRoomImpl.this) { lastPresenceSent = packet.asBuilder((String) null).removeExtension( MUCInitialPresence.ELEMENT, @@ -387,8 +423,7 @@ private void resetRoleForOccupant(EntityFullJid occupantJid) } else { - logger.error( - "Role reset for: " + occupantJid + " who does not exist"); + logger.error("Role reset for: " + occupantJid + " who does not exist"); } } } @@ -426,10 +461,7 @@ public ChatMemberImpl getChatMember(EntityFullJid occupantJid) return null; } - synchronized (members) - { - return members.get(occupantJid); - } + return members.get(occupantJid); } @Override