On Mon, 31 Jul 2023 16:52:00 GMT, Andy Goryachev <[email protected]> wrote:
>> This PR adds a file lock to the cache directory, allowing access from
>> multiple processes (that is, from more than one JVM) to that directory.
>>
>> This helps solving the issue where the cache is empty or doesn't exist yet,
>> and two JavaFX applications start at the same time, and both try to copy the
>> same native libraries to the same cache.
>>
>> No tests have been added to the PR though, since this requires a complex
>> scenario: building the SDK and publishing to local Maven repository,
>> creating two simple JavaFX applications that use those artifacts, using a
>> temporary folder for cache (via `javafx.cachedir`, and launching in parallel
>> both applications.
>>
>> I've tested successfully such scenario on Linux, Mac and Windows.
>
> modules/javafx.graphics/src/main/java/com/sun/glass/utils/NativeLibLoader.java
> line 311:
>
>> 309: FileLock fileLock = fileChannel.lock()) {
>> 310: try {
>> 311: if (!Files.exists(path)) {
>
> This code would fail if the target file exists but has a 0 length, or any
> length other that the actual resource length, or with the right length but
> wrong content.
> Do we want to address these scenarios?
This is a different type of a problem, that imho should be addressed as well,
but in a separate issue which also contains a test strategy. This is something
that requires a hash-based solution, but it is something that needs to be done
with great care, so I personally wouldn't rush this into this PR (as it might
cause criticial regression if not done correctly).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1188#discussion_r1280187329
