[MNG-7402] BuildListCalculator never detaches the classloader

[jira-importer]

Christoph Läubrich opened MNG-7402 and commented

maven/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java

Line 63 in 6b60710

BuilderCommon.attachToThread( project ); // Not totally sure if this is needed for anything

Her there is attachToThread called but never detached. This seems to be a leak, at least the threads current contextclassloader is never restored.

---

Affects: 3.8.4

Issue Links:

• MNG-7429 The classloader containing build extensions should be used throughout the build
  ("causes")

Remote Links:

• GitHub Pull Request #683
  
• GitHub Pull Request #690
  

Backported to: 4.0.0-alpha-2, 3.8.5

[jira-importer]

Michael Osipov commented

Like this?

diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java
index 9000c9301e..c35ac4134c 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java
+++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java
@@ -60,10 +60,18 @@ public ProjectBuildList calculateProjectBuilds( MavenSession session, List<TaskS
             }
             for ( MavenProject project : projects )
             {
-                BuilderCommon.attachToThread( project ); // Not totally sure if this is needed for anything
-                MavenSession copiedSession = session.clone();
-                copiedSession.setCurrentProject( project );
-                projectBuilds.add( new ProjectSegment( project, taskSegment, copiedSession ) );
+                ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+                try
+                {
+                    BuilderCommon.attachToThread( project ); // Not totally sure if this is needed for anything
+                    MavenSession copiedSession = session.clone();
+                    copiedSession.setCurrentProject( project );
+                    projectBuilds.add( new ProjectSegment( project, taskSegment, copiedSession ) );
+                }
+                finally
+                {
+                    Thread.currentThread().setContextClassLoader( tccl );
+                }
             }
         }
         return new ProjectBuildList( projectBuilds );

[jira-importer]

Christoph Läubrich commented

yes, even though the reset of the Threads CCL could be moved outside of the loop

[jira-importer]

Michael Osipov commented

Can you think of an IT which can depict the faulty behavior. I am not a CL expert to make proper judgement.

gnodet, Robert Scholte, any opinions applying such a patch on 3.8+?

[jira-importer]

Christoph Läubrich commented

This is all derived by code-review and I think its quite hard to reproduce / check this as a lot of tccl-action haven within maven so it might even be irrelevant here, but the general pattern should be as you have described that the original TCC is restored after a certain action has taken place to not leak classloaders if the thread is reused.

In this example, the current TCC wil be the one of the last Maven project after this method is called, this does not seem correct to me as the order of projects should not influence classlaoding.

[jira-importer]

Christoph Läubrich commented

Maybe in BuildListCalculatorTest.testCalculateProjectBuilds() one could add a check that the original classlaoder is restored afterwards.... Also LifecycleTaskSegmentCalculatorImplTest.testCalculateProjectBuilds() call the method and could be a place to check that the TCC is restored correctly.

Beside that the code is called in org.apache.maven.lifecycle.internal.LifecycleStarter.execute(MavenSession) where after the call the current TCC is passed to another call as the "original" classlaoder.

[jira-importer]

Michael Osipov commented

Please propose a PR if you want to have it in 3.8.5.

[jira-importer]

Michael Osipov commented

Fixed with e327be3d85918a23a5ba48d752143a6dbf8b83f7 and with d29af9018b5c80177102aaab96bd37ed5fedf26c for maven-3.8.x branch.