On Tue, 5 Mar 2024 11:07:13 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Ensure access to the `filesLoader` field of `BasicDirectoryModel` is >> synchronized. >> >> Without synchronization, a thread checks if `filesLoader` is not null and >> creates a new `FilesLoader` thread. If the thread is pre-empted between >> these two operations, another thread or even several threads can see the >> `null` value and create new `FilesLoader` threads. >> >> The same way, `BasicDirectoryModel.invalidateFileCache` needs to be >> synchornized to interrupt the current `filesLoader` and assign `null`. >> >> This bug allows reproducing `ConcurrentModificationException` seen in >> [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670) and >> [JDK-8307091](https://bugs.openjdk.org/browse/JDK-8307091) using the test in >> PR #18109. > > Alexey Ivanov has updated the pull request incrementally with four additional > commits since the last revision: > > - Replace synchronized invalidateFileCache with synchronized block inside > - Declare DoChangeContents constructor private, wrap its parameters > - Space after synchronized in DoChangeContents.run > - Convert runnable to local variable src/java.desktop/share/classes/javax/swing/plaf/basic/BasicDirectoryModel.java line 397: > 395: if (!fileCache.equals(newFileCache)) { > 396: if (loadThread.isInterrupted()) { > 397: return null; This should've been `return null` all the way. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18111#discussion_r1512665847