On Wed, 24 Jan 2024 16:51:15 GMT, Tejesh R <t...@openjdk.org> wrote:

> Using `synchronized (fileCache)` inside `ShellFolder.invoke` would be better 
> and one solution right? Than making local copy and again doing sanity 
> checks/changing to `ArrayList`?

I'm afraid I can't answer this question without more details on what is 
achieved by this code. We need to look closely into what was done for 
[JDK-8240690](https://bugs.openjdk.org/browse/JDK-8240690): _Race condition 
between EDT and BasicDirectoryModel.FilesLoader.run0()_.

Putting the entire method into `synchronized (fileCache)` is an easy solution. 
Yet any other thread which accesses `fileCache` will be blocked until the code 
exits the synchronized block in the `call` method. This somewhat defeats 
updating the file list in the background, doesn't it?

Even if you'll go this route, I'm for replacing `Vector` with `ArrayList` for 
the `newFileCache` and `newFiles` variables. These are local variables, they're 
not accessed concurrently. Yet they're accessed from two threads: the current 
one and the one where `ShellFolder.invoke` runs, so there could be a need to 
use another synchronisation technique to ensure thread-safety between these two 
threads.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17462#discussion_r1465414103

Reply via email to