You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2026/08/07 16:27:13 UTC
Re: [PR] [MNG-12594] Fix LegacySupport.setSession() thread safety in DefaultMaven [maven]
elharo commented on code in PR #12634:
URL: https://github.com/apache/maven/pull/12634#discussion_r3737233412
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result, chainedWorkspaceReader);
+ } finally {
+ legacySupport.setSession(previousSession);
Review Comment:
Fixed: `legacySupport.setSession(session)` is now inside the try block, so the previous session is restored even if setSession throws after previousSession is captured.
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result, chainedWorkspaceReader);
+ } finally {
+ legacySupport.setSession(previousSession);
Review Comment:
Fixed: setSession(session) is now inside the try block, so the previous session is restored even if setSession throws.
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result, chainedWorkspaceReader);
Review Comment:
Fixed: added testThatLegacySupportSessionIsRestoredAfterExecute in DefaultMavenTest, which pre-sets a session, runs maven.execute(), and asserts the pre-set session is restored afterward. Verified the test fails on the previous code (which unconditionally called setSession(null)).
--
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