Fix ConcurrentModificationException. (#984)

* Fix/supress warnings. * fix: Set XML parsing settings. * ref: Reuse ParticipantInfo.toEndpoint. * fix: Fix ConcurrentModificationException when bridge count changes.
jitsi · Oct 3, 2022 · 6537921 · 6537921
1 parent 1e258e1
commit 6537921
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 33 deletions.
diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/conference/source/Source.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/conference/source/Source.kt
@@ -95,7 +95,7 @@ data class Source(
         put("media_type", mediaType.toString())
         put("name", name ?: "null")
         put("msid", msid ?: "null")
-        put("videoType", videoType.toString() ?: "null")
+        put("videoType", videoType.toString())
     }
 
     companion object {

diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/colibri/Colibri2Session.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/colibri/Colibri2Session.kt
@@ -25,7 +25,6 @@ import org.jitsi.utils.OrderedJsonObject
 import org.jitsi.utils.logging2.Logger
 import org.jitsi.utils.logging2.createChildLogger
 import org.jitsi.xmpp.extensions.colibri.WebSocketPacketExtension
-import org.jitsi.xmpp.extensions.colibri2.Capability
 import org.jitsi.xmpp.extensions.colibri2.Colibri2Endpoint
 import org.jitsi.xmpp.extensions.colibri2.Colibri2Relay
 import org.jitsi.xmpp.extensions.colibri2.ConferenceModifiedIQ
@@ -77,19 +76,7 @@ internal class Colibri2Session(
     internal fun sendAllocationRequest(participant: ParticipantInfo): StanzaCollector {
 
         val request = createRequest(!created)
-        val endpoint = Colibri2Endpoint.getBuilder().apply {
-            setId(participant.id)
-            setCreate(true)
-            if (participant.sources.isNotEmpty()) {
-                setSources(participant.sources.toColibriMediaSources())
-            }
-            setStatsId(participant.statsId)
-            if (participant.supportsSourceNames) {
-                addCapability(Capability.CAP_SOURCE_NAME_SUPPORT)
-            }
-            if (participant.useSsrcRewriting) {
-                addCapability(Capability.CAP_SSRC_REWRITING_SUPPORT)
-            }
+        val endpoint = participant.toEndpoint(create = true, expire = false).apply {
             if (participant.audioMuted || participant.videoMuted) {
                 setForceMute(participant.audioMuted, participant.videoMuted)
             }
@@ -417,7 +404,7 @@ internal class Colibri2Session(
                 setId(id)
             }
             val endpoints = Endpoints.getBuilder()
-            endpoints.addEndpoint(participant.toEndpoint(create = create, expire = false))
+            endpoints.addEndpoint(participant.toEndpoint(create = create, expire = false).build())
             relay.setEndpoints(endpoints.build())
             request.addRelay(relay.build())
             sendRequest(request.build(), "Relay.updateParticipant")
@@ -429,7 +416,7 @@ internal class Colibri2Session(
             val relay = Colibri2Relay.getBuilder().apply { setId(id) }
             val endpoints = Endpoints.getBuilder()
 
-            participants.forEach { endpoints.addEndpoint(it.toEndpoint(create = false, expire = true)) }
+            participants.forEach { endpoints.addEndpoint(it.toEndpoint(create = false, expire = true).build()) }
 
             relay.setEndpoints(endpoints.build())
             request.addRelay(relay.build())
@@ -461,7 +448,7 @@ internal class Colibri2Session(
             )
 
             val endpoints = Endpoints.getBuilder()
-            participants.forEach { endpoints.addEndpoint(it.toEndpoint(create = true, expire = false)) }
+            participants.forEach { endpoints.addEndpoint(it.toEndpoint(create = true, expire = false).build()) }
             relay.setEndpoints(endpoints.build())
 
             relay.setTransport(

diff --git a/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/colibri/Extensions.kt b/jicofo-selector/src/main/kotlin/org/jitsi/jicofo/bridge/colibri/Extensions.kt
@@ -105,7 +105,7 @@ internal fun ParticipantInfo.toEndpoint(
     create: Boolean,
     /** Whether the request should have the "expire" flag set. */
     expire: Boolean
-): Colibri2Endpoint = Colibri2Endpoint.getBuilder().apply {
+): Colibri2Endpoint.Builder = Colibri2Endpoint.getBuilder().apply {
     setId(id)
     if (create) {
         setCreate(true)
@@ -124,7 +124,7 @@ internal fun ParticipantInfo.toEndpoint(
     if (expire) {
         setExpire(true)
     }
-}.build()
+}
 
 internal fun AbstractXMPPConnection.sendIqAndHandleResponseAsync(iq: IQ, block: (IQ?) -> Unit) {
     val stanzaCollector = createStanzaCollectorAndSend(iq)

diff --git a/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java b/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java
@@ -187,7 +187,7 @@ public class JitsiMeetConferenceImpl
     /**
      * The conference properties that we advertise in presence in the XMPP MUC.
      */
-    private final ConferenceProperties conferenceProperties = new ConferenceProperties();
+    private final ConcurrentHashMap<String, String> conferenceProperties = new ConcurrentHashMap<>();
 
     /**
      * See {@link JitsiMeetConference#includeInStatistics()}
@@ -512,7 +512,7 @@ else if (ConferenceConfig.config.enableAutoOwner())
             Boolean.TRUE.toString(),
             false);
 
-        presenceExtensions.add(ConferenceProperties.clone(conferenceProperties));
+        presenceExtensions.add(createConferenceProperties());
 
         // updates presence with presenceExtensions and sends it
         chatRoom.modifyPresence(null, presenceExtensions);
@@ -524,7 +524,7 @@ else if (ConferenceConfig.config.enableAutoOwner())
      * @param key the key of the property.
      * @param value the value of the property.
      */
-    private void setConferenceProperty(String key, String value)
+    private void setConferenceProperty(@NotNull String key, @NotNull String value)
     {
         setConferenceProperty(key, value, true);
     }
@@ -539,15 +539,22 @@ private void setConferenceProperty(String key, String value)
      * and {@code false} to only add the property locally. This is useful to
      * allow updating multiple properties but sending a single presence update.
      */
-    private void setConferenceProperty(String key, String value, boolean updatePresence)
+    private void setConferenceProperty(@NotNull String key, @NotNull String value, boolean updatePresence)
     {
-        conferenceProperties.put(key, value);
-        if (updatePresence && chatRoom != null)
+        String oldValue = conferenceProperties.put(key, value);
+        if (updatePresence && chatRoom != null && !value.equals(oldValue))
         {
-            chatRoom.setPresenceExtension(ConferenceProperties.clone(conferenceProperties), false);
+            chatRoom.setPresenceExtension(createConferenceProperties(), false);
         }
     }
 
+    private ConferenceProperties createConferenceProperties()
+    {
+        ConferenceProperties conferenceProperties = new ConferenceProperties();
+        this.conferenceProperties.forEach(conferenceProperties::put);
+        return conferenceProperties;
+    }
+
     /**
      * Process the new number of audio senders reported by the chat room.
      */
@@ -1564,10 +1571,7 @@ public OrderedJsonObject getDebugState()
             o.put("colibri_session_manager", colibriSessionManager.getDebugState());
         }
         OrderedJsonObject conferencePropertiesJson = new OrderedJsonObject();
-        for (ConferenceProperties.ConferenceProperty conferenceProperty : conferenceProperties.getProperties())
-        {
-            conferencePropertiesJson.put(conferenceProperty.getKey(), conferenceProperty.getValue());
-        }
+        conferenceProperties.forEach(conferencePropertiesJson::put);
         o.put("conference_properties", conferencePropertiesJson);
         o.put("include_in_statistics", includeInStatistics);
         o.put("conference_sources", conferenceSources.toJson());

diff --git a/jicofo/src/main/java/org/jitsi/jicofo/rest/Debug.java b/jicofo/src/main/java/org/jitsi/jicofo/rest/Debug.java
@@ -61,7 +61,8 @@ public String confDebug(@PathParam("confId") String confId)
     @Path("/conferences")
     @Produces(MediaType.APPLICATION_JSON)
     @NotNull
-    public String conferences(@PathParam("confId") String confId)
+    @SuppressWarnings("unchecked")
+    public String conferences()
     {
         JSONArray conferencesJson = new JSONArray();
         for (JitsiMeetConference c : jicofoServices.getFocusManager().getAllConferences())

diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/MuteIqHandler.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/MuteIqHandler.kt
@@ -88,7 +88,7 @@ private fun handleRequest(request: MuteRequest): IqProcessingResult {
     val jidToMute = request.jidToMute
     val doMute = request.doMute
     val mediaType = request.mediaType
-    if (doMute == null || jidToMute == null || mediaType == null) {
+    if (doMute == null || jidToMute == null) {
         logger.warn("Mute request missing required fields: ${request.iq.toXML()}")
         return RejectedWithError(request.iq, StanzaError.Condition.bad_request)
     }

diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt
@@ -57,6 +57,12 @@ import org.jivesoftware.smack.provider.ProviderManager
 import org.jivesoftware.smackx.bytestreams.socks5.Socks5Proxy
 
 fun initializeSmack() {
+    System.setProperty("jdk.xml.entityExpansionLimit", "0")
+    System.setProperty("jdk.xml.maxOccurLimit", "0")
+    System.setProperty("jdk.xml.elementAttributeLimit", "524288")
+    System.setProperty("jdk.xml.totalEntitySizeLimit", "0")
+    System.setProperty("jdk.xml.maxXMLNameLimit", "524288")
+    System.setProperty("jdk.xml.entityReplacementLimit", "0")
     SmackConfiguration.setDefaultReplyTimeout(15000)
     // if there is a parsing error, do not break the connection to the server(the default behaviour) as we need it for
     // the other conferences.