gnodet-bot commented on code in PR #12695:
URL: https://github.com/apache/maven/pull/12695#discussion_r4051139151
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultSession.java:
##########
@@ -63,6 +65,7 @@ public class DefaultSession extends AbstractSession
implements InternalMavenSess
private final MavenRepositorySystem mavenRepositorySystem;
private final RuntimeInformation runtimeInformation;
private final Map<String, Project> allProjects = new ConcurrentHashMap<>();
+ private BuildEnvironment buildEnvironment;
Review Comment:
**[Medium] Missing `volatile` on lazily-initialized field — data race in a
`@ThreadSafe` class.**
`DefaultSession` implements `Session`, which carries `@ThreadSafe`. The new
`buildEnvironment` field is lazily initialized with a plain check-then-act:
```java
if (buildEnvironment == null) {
buildEnvironment =
BuildReportCollector.buildEnvironment(getMavenSession());
}
return buildEnvironment;
```
Without `volatile`, the JMM offers no happens-before guarantee between the
write in thread A and the read in thread B. Thread B may observe a stale `null`
and re-compute, or (worse) observe a partially-constructed
`DefaultBuildEnvironment` record. Note that `BuildReportCollector` itself uses
`private volatile BuildEnvironment buildEnvironment` on its own caching field —
the same pattern, done correctly.
Fix: add `volatile`.
```suggestion
private volatile BuildEnvironment buildEnvironment;
```
--
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]
