[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8327137: Add test for ConcurrentModificationException in BasicDirectoryModel [v3]
From:       Sergey Bylokhov <serb () openjdk ! org>
Date:       2024-03-31 22:42:35
Message-ID: ef8Qpm3xuLRZaLR-M4EU1SyJNeoyc6DKDYdkRh2A17E=.a183d12a-cd4f-4864-85a5-675c04710381 () github ! com
[Download RAW message or body]

On Fri, 29 Mar 2024 20:02:53 GMT, Alexey Ivanov <aivanov@openjdk.org> wrote:

> > > However, there are cases where instantiating and testing is safe on main \
> > > thread.
> > 
> > That is my point, make it less safe - when the component is created on one \
> > thread:EDT and then for some reason accessed on a different thread, the \
> > rescanCurrentDirectory should still work.
> 
> > > However, there are cases where instantiating and testing is safe on main \
> > > thread.
> > 
> > That is my point, make it less safe - when the component is created on one \
> > thread:EDT and then for some reason accessed on a different thread, the \
> > rescanCurrentDirectory should still work.
> 
> I got your idea!
> 
> However, I still see no benefit for doing it.
> 
> Yes, when `JFileChooser` is instantiated, it creates its UI delegates and the \
> default model, `BasicDirectoryModel`. The constructor of the model calls \
> `validateFileCache` which starts the background thread and will update `fileCache`. \
>  It does not make the initialisation less thread-safe: the same two threads are \
> involved — `FilesLoader` and EDT. Yet there's no race yet… 
> The `ConcurrentModificationException` is thrown when the background `FilesLoader` \
> is iterating over `fileCache` while EDT runs `DoChangeContents.run` which adds \
> elements to `fileCache` or removes elements from it. 
> By the time the `Scanner` threads start, a race becomes possible. If the initial \
> `FilesLoader` completes and posts the `DoChangeContents` object to EDT, the \
> `FilesLoader` threads created by the scanners will race… but `fileCache` is empty \
> initially, thus iterating over it is very quick. 
> Therefore getting `ConcurrentModificationException` is unlikely until `fileCache` \
> contains a list of files. This state is reached later in the timeline of the test. 
> The initial `FilesLoader` thread that's started when `JFileChooser` is instantiated \
> plays a role too… Yet it's running concurrently with the `Scanner` threads \
> whether the `JFileChooser` object is created on the current thread or on EDT. 
> ---
> 
> Taking the above into account, instantiating `JFileChooser` on EDT adds complexity \
> to the test but brings no benefits.

ok, I agree with you.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18109#discussion_r1545858033


[prev in list] [next in list] [prev in thread] [next in thread]