[MNG-7487] Fix deadlock during forked lifecycle executions

gnodet · gnodet · commit 0b79a9e64325 · 2022-05-30T13:08:27.000+02:00
diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java
@@ -85,6 +85,8 @@ public class MojoExecutor
 
     private final ReadWriteLock aggregatorLock = new ReentrantReadWriteLock();
 
+    private final Map<Thread, MojoDescriptor> mojos = new ConcurrentHashMap<>();
+
     public MojoExecutor()
     {
     }
@@ -206,10 +208,7 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
             }
         }
 
-        try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor, aggregatorLock ) )
-        {
-            doExecute( session, mojoExecution, projectIndex, dependencyContext );
-        }
+        doExecute( session, mojoExecution, projectIndex, dependencyContext );
     }
 
     /**
@@ -220,13 +219,14 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
      * TODO: ideally, the builder should take care of the ordering in a smarter way
      * TODO: and concurrency issues fixed with MNG-7157
      */
-    private static class ProjectLock implements AutoCloseable
+    private class ProjectLock implements AutoCloseable
     {
         final Lock acquiredAggregatorLock;
         final Lock acquiredProjectLock;
 
-        ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )
+        ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
         {
+            mojos.put( Thread.currentThread(), mojoDescriptor );
             if ( session.getRequest().getDegreeOfConcurrency() > 1 )
             {
                 boolean aggregator = mojoDescriptor.isAggregator();
@@ -254,6 +254,7 @@ public void close()
             {
                 acquiredAggregatorLock.unlock();
             }
+            mojos.remove( Thread.currentThread() );
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
@@ -292,8 +293,23 @@ private void doExecute( MavenSession session, MojoExecution mojoExecution, Proje
 
         ensureDependenciesAreResolved( mojoDescriptor, session, dependencyContext );
 
-        eventCatapult.fire( ExecutionEvent.Type.MojoStarted, session, mojoExecution );
+        try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor ) )
+        {
+            doExecute2( session, mojoExecution );
+        }
+        finally
+        {
+            for ( MavenProject forkedProject : forkedProjects )
+            {
+                forkedProject.setExecutionProject( null );
+            }
+        }
+    }
 
+    private void doExecute2( MavenSession session, MojoExecution mojoExecution )
+            throws LifecycleExecutionException
+    {
+        eventCatapult.fire( ExecutionEvent.Type.MojoStarted, session, mojoExecution );
         try
         {
             try
@@ -314,13 +330,6 @@ private void doExecute( MavenSession session, MojoExecution mojoExecution, Proje
 
             throw e;
         }
-        finally
-        {
-            for ( MavenProject forkedProject : forkedProjects )
-            {
-                forkedProject.setExecutionProject( null );
-            }
-        }
     }
 
     public void ensureDependenciesAreResolved( MojoDescriptor mojoDescriptor, MavenSession session,

Original file line number	Diff line number	Diff line change
`@@ -85,6 +85,8 @@ public class MojoExecutor`
`85`	`85`
`86`	`86`	`private final ReadWriteLock aggregatorLock = new ReentrantReadWriteLock();`
`87`	`87`
	`88`	`+ private final Map<Thread, MojoDescriptor> mojos = new ConcurrentHashMap<>();`
	`89`	`+`
`88`	`90`	`public MojoExecutor()`
`89`	`91`	`{`
`90`	`92`	`}`
`@@ -206,10 +208,7 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project`
`206`	`208`	`}`
`207`	`209`	`}`
`208`	`210`
`209`		`- try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor, aggregatorLock ) )`
`210`		`- {`
`211`		`- doExecute( session, mojoExecution, projectIndex, dependencyContext );`
`212`		`- }`
	`211`	`+ doExecute( session, mojoExecution, projectIndex, dependencyContext );`
`213`	`212`	`}`
`214`	`213`
`215`	`214`	`/**`
`@@ -220,13 +219,14 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project`
`220`	`219`	`* TODO: ideally, the builder should take care of the ordering in a smarter way`
`221`	`220`	`* TODO: and concurrency issues fixed with MNG-7157`
`222`	`221`	`*/`
`223`		`- private static class ProjectLock implements AutoCloseable`
	`222`	`+ private class ProjectLock implements AutoCloseable`
`224`	`223`	`{`
`225`	`224`	`final Lock acquiredAggregatorLock;`
`226`	`225`	`final Lock acquiredProjectLock;`
`227`	`226`
`228`		`- ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )`
	`227`	`+ ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )`
`229`	`228`	`{`
	`229`	`+ mojos.put( Thread.currentThread(), mojoDescriptor );`
`230`	`230`	`if ( session.getRequest().getDegreeOfConcurrency() > 1 )`
`231`	`231`	`{`
`232`	`232`	`boolean aggregator = mojoDescriptor.isAggregator();`
`@@ -254,6 +254,7 @@ public void close()`
`254`	`254`	`{`
`255`	`255`	`acquiredAggregatorLock.unlock();`
`256`	`256`	`}`
	`257`	`+ mojos.remove( Thread.currentThread() );`
`257`	`258`	`}`
`258`	`259`
`259`	`260`	`@SuppressWarnings( { "unchecked", "rawtypes" } )`
`@@ -292,8 +293,23 @@ private void doExecute( MavenSession session, MojoExecution mojoExecution, Proje`
`292`	`293`
`293`	`294`	`ensureDependenciesAreResolved( mojoDescriptor, session, dependencyContext );`
`294`	`295`
`295`		`- eventCatapult.fire( ExecutionEvent.Type.MojoStarted, session, mojoExecution );`
	`296`	`+ try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor ) )`
	`297`	`+ {`
	`298`	`+ doExecute2( session, mojoExecution );`
	`299`	`+ }`
	`300`	`+ finally`
	`301`	`+ {`
	`302`	`+ for ( MavenProject forkedProject : forkedProjects )`
	`303`	`+ {`
	`304`	`+ forkedProject.setExecutionProject( null );`
	`305`	`+ }`
	`306`	`+ }`
	`307`	`+ }`
`296`	`308`
	`309`	`+ private void doExecute2( MavenSession session, MojoExecution mojoExecution )`
	`310`	`+ throws LifecycleExecutionException`
	`311`	`+ {`
	`312`	`+ eventCatapult.fire( ExecutionEvent.Type.MojoStarted, session, mojoExecution );`
`297`	`313`	`try`
`298`	`314`	`{`
`299`	`315`	`try`
`@@ -314,13 +330,6 @@ private void doExecute( MavenSession session, MojoExecution mojoExecution, Proje`
`314`	`330`
`315`	`331`	`throw e;`
`316`	`332`	`}`
`317`		`- finally`
`318`		`- {`
`319`		`- for ( MavenProject forkedProject : forkedProjects )`
`320`		`- {`
`321`		`- forkedProject.setExecutionProject( null );`
`322`		`- }`
`323`		`- }`
`324`	`333`	`}`
`325`	`334`
`326`	`335`	`public void ensureDependenciesAreResolved( MojoDescriptor mojoDescriptor, MavenSession session,`