search

zsmartsystems / com.zsmartsystems.zigbee

ZigBee Cluster Library Java framework supporting multiple dongles
Eclipse Public License 1.0
142 stars 88 forks source link

ConcurrentModificationException while iterating queues #1365

Closed mikomarrache closed 1 year ago

mikomarrache commented 1 year ago

We found other issues related to concurrency:

java.util.ConcurrentModificationException: null
    at java.util.ArrayDeque$DeqIterator.next(ArrayDeque.java:648)
    at com.zsmartsystems.zigbee.transaction.ZigBeeTransactionQueue.shutdown(ZigBeeTransactionQueue.java:137)
    at com.zsmartsystems.zigbee.transaction.ZigBeeTransactionManager.removeNode(ZigBeeTransactionManager.java:607)
    at com.zsmartsystems.zigbee.transaction.ZigBeeTransactionManager.nodeRemoved(ZigBeeTransactionManager.java:786)
    at com.zsmartsystems.zigbee.ZigBeeNetworkManager$10.run(ZigBeeNetworkManager.java:1689)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
java.util.ConcurrentModificationException: null
    at java.util.HashMap$HashIterator.nextNode(HashMap.java:1445)
    at java.util.HashMap$KeyIterator.next(HashMap.java:1469)
    at com.zsmartsystems.zigbee.transaction.ZigBeeTransactionManager.removeNode(ZigBeeTransactionManager.java:613)
    at com.zsmartsystems.zigbee.transaction.ZigBeeTransactionManager.nodeRemoved(ZigBeeTransactionManager.java:786)
    at com.zsmartsystems.zigbee.ZigBeeNetworkManager$10.run(ZigBeeNetworkManager.java:1689)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

We are investigating how to solve it.

mikomarrache commented 1 year ago

https://github.com/zsmartsystems/com.zsmartsystems.zigbee/blob/0c393dbb7df447522347ad5bfd7352a94dc68161/com.zsmartsystems.zigbee/src/main/java/com/zsmartsystems/zigbee/transaction/ZigBeeTransactionQueue.java#L69

Regarding the first exception, should we replace the ArrayDeque by ConcurrentLinkedDeque ?

cdjackson commented 1 year ago

Agreed - I think using ConcurrentLinkedDeque probably makes sense here so please feel free to provide a PR.

mikomarrache commented 1 year ago

Regarding the second exception, it seems that cancelling the transaction somehow modifies the outstandingTransactions set.

Since synchronized blocks are re-entrant, modifying the set while iterating is possible.

I'm not sure yet how to solve it though.

cdjackson commented 1 year ago

This should be closed with #1375