elharo opened a new issue, #173:
URL: https://github.com/apache/maven-toolchains-plugin/issues/173

   ## Summary
   
   `ToolchainDiscoverer.getToolchainModel()` is called from a 
`parallelStream()` in `discoverToolchains()`. The `cacheModified` volatile 
field is written concurrently by multiple threads without atomic coordination, 
which can cause cache updates to be lost.
   
   ## Location
   
   `ToolchainDiscoverer.java:217-227`
   
   
https://github.com/apache/maven-toolchains-plugin/blob/master/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java#L217-L227
   
   ## Code
   
   ```java
   ToolchainModel getToolchainModel(Path jdk) {
       ToolchainModel model = cache.get(jdk);
       if (model == null) {
           model = doGetToolchainModel(jdk);
           if (model != null) {
               cache.put(jdk, model);
               cacheModified = true;
           }
       }
       return model;
   }
   ```
   
   Called from `discoverToolchains()` via parallel stream:
   
   ```java
   List<ToolchainModel> tcs = jdks.parallelStream()
           .map(s -> {
               ToolchainModel tc = getToolchainModel(s);
               // ...
           })
           .sorted(...)
           .collect(Collectors.toList());
   writeCache();
   ```
   
   ## Problem
   
   1. `cacheModified = true` is written from multiple threads in the parallel 
stream without synchronization
   2. After the parallel stream completes, `writeCache()` reads 
`cacheModified`, writes the cache file, and sets it back to `false`
   3. If a thread writes `cacheModified = true` after `writeCache()` has read 
it but before it sets it to `false`, the cache update is silently lost, and the 
newly discovered JDK won't be persisted
   4. The `ConcurrentHashMap` itself is fine (thread-safe), but the 
coordination between `cacheModified` and the cache file write is not atomic
   
   ## Impact
   
   Under concurrent load (which is the default for `parallelStream()`), some 
discovered JDK toolchains may not be persisted to the cache file. This means 
`writeCache()` may be called with `cacheModified == false` even though new 
entries were added during the stream.
   
   ## Suggested Fix
   
   Use an `AtomicBoolean` for `cacheModified` or re-check cache for new entries 
at write time:
   
   ```java
   private final AtomicBoolean cacheModified = new AtomicBoolean();
   
   // In getToolchainModel:
   cacheModified.set(true);
   
   // In writeCache:
   cacheModified.set(false);
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to