igniterealtime
ChatContainer: avoid allocation of ArrayList
The allocation might be needed (I guess) to avoid concurrency problems. The chatRoomList is changed in two places: in the addChatRoom() that is synchronized and cleanupChatRoom() that is not synchronized. So just to be sure I made it synchronized too.
It looks like some pattern or remains after some refactoring because there are other places with such iteration over an ArrayList copy: org/jivesoftware/spark/SessionManager.java:206 org/jivesoftware/spark/filetransfer/SparkTransferManager.java:695 org/jivesoftware/spark/ui/ChatRoom.java:1139 org/jivesoftware/spark/ui/ContactGroup.java:270 org/jivesoftware/spark/ui/ContactGroup.java:284 org/jivesoftware/spark/ui/ContactGroup.java:416 org/jivesoftware/spark/ui/ContactList.java:379
Does that mean that this should go to draft, or should this be tackled in another PR?
It creates a copy of the list (new ArrayList<>(this.presenceListeners)) to avoid concurrent modification issues if a listener modifies the list during iteration.
I can't replace all the places where this happens but for listeners we have a few options:
- Use Swing's EventListenerList that is just a synchronized list.
- Use CopyOnWriteArrayList and it is recommended https://stackoverflow.com/a/36571559/1049542
The CopyOnWriteArrayList is already used in some places for listeners also an AI recommended to use it.
So I changed all the places where I found a list of listeners to the CopyOnWriteArrayList so that same pattern will be used everywhere. I can't properly test it but Sparks looks working fine.