[I] [maven-4.0.x] DefaultProfileInjector: non-atomic computeIfAbsent on synchronized WeakHashMap [maven]

["elharo (via GitHub)" <gi...@apache.org>]

elharo opened a new issue, #12591:
URL: https://github.com/apache/maven/issues/12591

   # DefaultProfileInjector: Non-atomic computeIfAbsent on synchronized WeakHashMap
   
   **Found in:** maven-4.0.x branch
   **File:** `impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java` (lines 56, 68-70)
   **Severity:** High
   
   ## Description
   
   `Collections.synchronizedMap()` only synchronizes individual method calls, but `computeIfAbsent()` is a compound operation. Since `WeakHashMap` does not override `computeIfAbsent()`, two threads can execute the inner map creation concurrently:
   
   ```java
   Model result = CACHE.computeIfAbsent(model, k -> new ConcurrentHashMap<>())
           .computeIfAbsent(profiles, l -> doInjectProfiles(model, profiles));
   ```
   
   One thread's inner `ConcurrentHashMap` can be discarded (and its profile results lost). This causes profile injection cache inconsistency under concurrent model building in multi-module reactor builds.


-- 
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: issues-unsubscribe@maven.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [maven-4.0.x] DefaultProfileInjector: non-atomic computeIfAbsent on synchronized WeakHashMap [maven]

["gnodet (via GitHub)" <gi...@apache.org>]

gnodet commented on issue #12591:
URL: https://github.com/apache/maven/issues/12591#issuecomment-5457537639

   Closing as invalid after investigation.
   
   The bug report's premise — that `WeakHashMap` doesn't override `computeIfAbsent` and therefore the operation falls through to a non-atomic default — is irrelevant because the **wrapper** (`Collections.synchronizedMap()`) does override `computeIfAbsent`.
   
   Since Java 9, `Collections.SynchronizedMap.computeIfAbsent()` acquires the `mutex` lock, then delegates to the underlying map's `computeIfAbsent()`, and releases the lock on exit. This makes the compound operation atomic with respect to all other operations on the same synchronized wrapper.
   
   The second `computeIfAbsent` operates on a `ConcurrentHashMap`, which is inherently thread-safe. No code change is needed.


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [maven-4.0.x] DefaultProfileInjector: non-atomic computeIfAbsent on synchronized WeakHashMap [maven]

["gnodet (via GitHub)" <gi...@apache.org>]

gnodet closed issue #12591: [maven-4.0.x] DefaultProfileInjector: non-atomic computeIfAbsent on synchronized WeakHashMap
URL: https://github.com/apache/maven/issues/12591


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org