parse-community / Parse-SDK-Android

The Android SDK for Parse Platform
https://parseplatform.org/
Other
1.88k stars 735 forks source link

ConcurrentModificationException in ParseTraverser #1061

Closed shlusiak closed 2 years ago

shlusiak commented 3 years ago

There is still the occasional ConcurrentModificationException in ParseTraverser.

Parse version: 1.25.0

Stack trace:

Fatal Exception: java.util.ConcurrentModificationException
       at java.util.HashMap$HashIterator.nextNode(HashMap.java:1441)
       at java.util.HashMap$KeyIterator.next(HashMap.java:1465)
       at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1084)
       at com.parse.ParseTraverser.traverseInternal(ParseTraverser.java:101)
       at com.parse.ParseTraverser.traverseInternal(ParseTraverser.java:95)
       at com.parse.ParseTraverser.traverseInternal(ParseTraverser.java:102)
       at com.parse.ParseTraverser.traverse(ParseTraverser.java:140)
       at com.parse.OfflineStore.saveLocallyAsync(OfflineStore.java:666)
       at com.parse.OfflineStore.access$1300(OfflineStore.java:36)

The issue appears that while ParseObject.keySet() returns a Collections.unmodifiableSet(), the set is a read-through to the underlying HashMap, which is not synchronised.

https://github.com/parse-community/Parse-SDK-Android/blob/797786ff42223094c20d2dadd7af9e616020175a/parse/src/main/java/com/parse/ParseObject.java#L1489-L1497

I believe we either need to make the estimatedData a synchronised Map, or we need to work on a copy of the set when traversing, to guard against concurrent modifications. Thoughts about making estimatedData a synchronized map?

https://github.com/parse-community/Parse-SDK-Android/blob/797786ff42223094c20d2dadd7af9e616020175a/parse/src/main/java/com/parse/ParseTraverser.java#L98-L103

mtrezza commented 2 years ago

Closing via #1062