RFR: 8323670: A few client tests intermittently throw ConcurrentModificationException
Alexey Ivanov
aivanov at openjdk.org
Wed Jan 24 19:28:30 UTC 2024
On Wed, 17 Jan 2024 12:55:52 GMT, Tejesh R <tr at openjdk.org> wrote:
> Suggested fix [JDK-8307091](https://bugs.openjdk.org/browse/JDK-8307091) also created concurrent exception intermittently (monthly once/quarterly once) on CI system. The issue was not able to be reproduced yet, hence proposing an alternative fix which uses iterators to compare the List.
> CI testing shows green.
In [JDK-8307091](https://bugs.openjdk.org/browse/JDK-8307091), `ConcurrentModificationException` is thrown from `equals`, likely from this line:
https://github.com/openjdk/jdk/blob/2a799e5c82395919b807561da4a062e0fe6da31d/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicDirectoryModel.java#L364
Here, `fileCache` is passed as a parameter, it can be modified while `newFileCache.subList().equals()` runs. You could've put this into `synchronized(fileCache)` to solve the problem.
In [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670), the exception is thrown from `Vector` constructor:
https://github.com/openjdk/jdk/blob/c702dcabf8befc2db2baf53655f20391ee5d2d09/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicDirectoryModel.java#L365
Here, we have exactly the same problem: `fileCache`, well `fileCache.subList` to be exact, is iterated without locking. (The `subList` uses the same object as the lock object, which is `fileCache`.) If you move this line into `synchronized(fileCache)`, the problem would resolved.
---
However, as I pointed out above, the code in the `call` method may still fail. There are at least two bugs where `ConcurrentModificationException` is thrown, which means `fileCache` can be modified. The `for`-loops access elements in `fileCache` using the variable `oldSize` as the size of the collection. Since the size can grow _or shrink_, it is _possible_ that the code requests an element which is not in the `fileCache` any more.
Yes, putting everything into `synchronized(fileCache)` will resolve this problem too. Is it the best solution?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17462#issuecomment-1908778630
More information about the client-libs-dev
mailing list