RFR: 8327137: Add test for ConcurrentModificationException in BasicDirectoryModel [v3]

Alexey Ivanov aivanov at openjdk.org
Fri Mar 29 20:06:32 UTC 2024


On Fri, 29 Mar 2024 02:00:18 GMT, Sergey Bylokhov <serb at 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.

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.

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

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


More information about the client-libs-dev mailing list