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

Reply via email to