I have a following spring bean (a little edited) This bean is responsible for adding/removing websocket connections to an internal connection (this.sockets). New connetions may be added/removed and are iterated each time a message needs to be sent.
As you can see every operation on the sockets
field is wrapped in a synchronized()
block it is private and is not used outside of this class, every use of this field is in the code i pasted below.
But still from time to time i am getting an ConcurrentModificartionException
that happens in the for loop in onApplicationEvent
call. This call may indeed be called by multiple threads in the application but my understanding was that if every call that actually modifies or iterates on this.socket
variable is wrapped in synchronized() block on the same object then each of these calls cannot be invoked in parallel and have to "Wait" for each other.
How it is possible that ConcurrentModificationException
is thrown in this code? Bear in mind that i am trying to understand the reason for exception i am getting and i am not looking for a quick fix that will just go around this issue (so answers like 'why dont you use XXX here' do not actually solve anything for me - i am trying to find out what i did wrong in my code).
The code:
public class MyClass extends TextWebSocketHandler implements ApplicationListener<AppEventMessage>{
private static final Logger log= LoggerFactory.getLogger(MyClass.class);
private Set<WebSocketSession> sockets=new HashSet<>();
@Override
public void afterConnectionEstablished(WebSocketSession session) throws Exception {
super.afterConnectionEstablished(session);
synchronized (this.sockets) {
this.sockets.add(session);
}
}
@Override
public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception {
synchronized (this.sockets) {
this.sockets.remove(session);
}
}
@Override
public void onApplicationEvent(AppEventMessage event) {
synchronized (this.sockets) {
for (WebSocketSession session : this.sockets) {
try {
log.info("sendin");
session.sendMessage(new TextMessage(event.getMessage()));
} catch (Throwable e) {
log.warn("error");
}
}
}
}
}
The most relevant part of stacktrace (with my comments)
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextNode(HashMap.java:1445)
at java.util.HashMap$KeyIterator.next(HashMap.java:1469)
at ...MyClass.onApplicationEvent(ApplicationEventWebsocketHandler.java:46) <- (line 46 is the for(WebSocketSession ...) line)
at ...MyClass.onApplicationEvent(ApplicationEventWebsocketHandler.java:19) <- (line 19 is the "public class Myclass" line - how it is possible?)
at org.springframework.context.event.SimpleApplicationEventMulticaster.invokeListener(SimpleApplicationEventMulticaster.java:163)
Agree with your diagnosis so far. There is a second way to get a ConcurrentModificationException
that is commonly overlooked...
Those synchronized
locks are re-entrant which means the thread holding the lock in one method can access all methods. So the thread causing your CME is quite possibly a callback from within the for
loop that ends up back in your connection-listener methods.
IMO, the most likely scenario is that the connection is only detected as closed when trying to sendMessage
.
Resolution: There are cleverer solutions but this is one that cannot go wrong:
// Take copy of sockets before iterating it to defend against self-induced ConcurrentModificationException
for (WebSocketSession session : new HashSet<>(this.sockets)) {