[maven-4.0.x] DefaultMaven: LegacySupport.setSession() thread safety in @Singleton

[elharo]

DefaultMaven: LegacySupport.setSession() thread safety in @singleton

Found in: maven-4.0.x branch
File: impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java (lines 225, 161)
Severity: High

Description

DefaultMaven is annotated @Singleton, meaning a single instance is shared across the application. LegacySupport is a thread-local-like holder for the current MavenSession. The setSession() calls at lines 225 and 161 are not synchronized:

// line 225:
legacySupport.setSession(session);

// line 161 (in finally):
legacySupport.setSession(null);

If execute() were called concurrently from multiple threads, calls to setSession() and setSession(null) from different threads would race, causing sessions to leak or be corrupted. While Maven is traditionally single-threaded per JVM, the @Singleton annotation combined with mutable instance state is a latent concurrency bug.

[gnodet]

Closing as invalid after investigation.

While DefaultMaven is indeed @Singleton, LegacySupport is already thread-safe via ThreadLocal:

DefaultLegacySupport (at impl/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultLegacySupport.java) stores the session in a private static final ThreadLocal<AtomicReference<MavenSession>> SESSION = new InheritableThreadLocal<>(). Both setSession() and getSession() operate exclusively on this ThreadLocal, so each calling thread has its own isolated storage. No synchronization is needed.

The LegacySupport interface's own Javadoc explicitly states the design intent: "Saving the session (in a thread-local variable) is our best effort to record any state."