Concurrency issue in ping failed listeners set

Daniele_Ricci · February 15, 2017, 1:31pm

Hello devs,

I believe there is a race condition when iterating over pingFailedListeners in PingManager:

java.util.ConcurrentModificationException at java.util.HashMap$HashIterator.nextEntry(HashMap.java:788) at java.util.HashMap$KeyIterator.next(HashMap.java:815) at org.jivesoftware.smackx.ping.PingManager.pingMyServer(PingManager.java:252) at org.kontalk.service.msgcenter.MessageCenterService$3.run(MessageCenterService.java:1114) at java.lang.Thread.run(Thread.java:818)

It’s a synchronized set and that’s ok, but access to it should be synchronized manually (source: Collections (Java Platform SE 7 ))

For reference: Smack concurrency issue for ping failed listener · Issue #955 · kontalk/androidclient · GitHub

Best

Flow · February 15, 2017, 7:17pm

Thanks, fixed with Use CopyOnWriteArraySet for pingFailedListeners · Flowdalic/Smack@6cfdf2b · GitHub

Daniele_Ricci · February 15, 2017, 8:33pm

Thank you for the quick fix. Any chance this might get into 4.1 as well?

Flow · February 16, 2017, 10:05am

Sadly not, i’ve have to stop supporting old version at some time. But nothing prevents you backporting the fix if you want to stay on 4.1. Just copy the PingManager code, fix it, and place it in a different package or rename the manager to e.g. PingManagerV2.

Daniele_Ricci · February 16, 2017, 10:07am

No problem, thank you. I’ll do that until I’ll switch to Smack 4.2.