You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2022/05/30 11:08:36 UTC

[maven] branch maven-3.8.x updated (88a03f8ea -> 5c0d6b9a1)

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a change to branch maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git


    from 88a03f8ea [MNG-7486] Create a multiline message helper for boxed log messages
     new 0b79a9e64 [MNG-7487] Fix deadlock during forked lifecycle executions
     new 5c0d6b9a1 [MNG-7476] Display a warning when an aggregator mojo is locking other mojos executions

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../maven/lifecycle/internal/MojoExecutor.java     | 110 ++++++++++++++++-----
 1 file changed, 86 insertions(+), 24 deletions(-)


[maven] 01/02: [MNG-7487] Fix deadlock during forked lifecycle executions

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 0b79a9e64325f21659923fd01606b26d4c993b63
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Thu May 12 10:04:51 2022 +0200

    [MNG-7487] Fix deadlock during forked lifecycle executions
---
 .../maven/lifecycle/internal/MojoExecutor.java     | 37 ++++++++++++++--------
 1 file changed, 23 insertions(+), 14 deletions(-)

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
index cf97c8cab..beca2eff0 100644
--- 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 @@ public class MojoExecutor
             }
         }
 
-        try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor, aggregatorLock ) )
-        {
-            doExecute( session, mojoExecution, projectIndex, dependencyContext );
-        }
+        doExecute( session, mojoExecution, projectIndex, dependencyContext );
     }
 
     /**
@@ -220,13 +219,14 @@ public class MojoExecutor
      * 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 class MojoExecutor
             {
                 acquiredAggregatorLock.unlock();
             }
+            mojos.remove( Thread.currentThread() );
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
@@ -292,8 +293,23 @@ public class MojoExecutor
 
         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 @@ public class MojoExecutor
 
             throw e;
         }
-        finally
-        {
-            for ( MavenProject forkedProject : forkedProjects )
-            {
-                forkedProject.setExecutionProject( null );
-            }
-        }
     }
 
     public void ensureDependenciesAreResolved( MojoDescriptor mojoDescriptor, MavenSession session,


[maven] 02/02: [MNG-7476] Display a warning when an aggregator mojo is locking other mojos executions

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 5c0d6b9a11a74493391ca5c1a3e78a5dc7dc0354
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Mon May 16 11:26:49 2022 +0200

    [MNG-7476] Display a warning when an aggregator mojo is locking other mojos executions
---
 .../maven/lifecycle/internal/MojoExecutor.java     | 73 +++++++++++++++++++---
 1 file changed, 63 insertions(+), 10 deletions(-)

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
index beca2eff0..3ef77844b 100644
--- 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
@@ -24,6 +24,7 @@ import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
 import org.apache.maven.artifact.resolver.filter.CumulativeScopeArtifactFilter;
 import org.apache.maven.execution.ExecutionEvent;
 import org.apache.maven.execution.MavenSession;
+import org.apache.maven.internal.MultilineMessageHelper;
 import org.apache.maven.lifecycle.LifecycleExecutionException;
 import org.apache.maven.lifecycle.MissingProjectException;
 import org.apache.maven.plugin.BuildPluginManager;
@@ -40,6 +41,8 @@ import org.codehaus.plexus.component.annotations.Component;
 import org.codehaus.plexus.component.annotations.Requirement;
 import org.codehaus.plexus.util.StringUtils;
 import org.eclipse.aether.SessionData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -52,7 +55,6 @@ import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -71,6 +73,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 public class MojoExecutor
 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger( MojoExecutor.class );
+
     @Requirement
     private BuildPluginManager pluginManager;
 
@@ -83,7 +87,7 @@ public class MojoExecutor
     @Requirement
     private ExecutionEventCatapult eventCatapult;
 
-    private final ReadWriteLock aggregatorLock = new ReentrantReadWriteLock();
+    private final OwnerReentrantReadWriteLock aggregatorLock = new OwnerReentrantReadWriteLock();
 
     private final Map<Thread, MojoDescriptor> mojos = new ConcurrentHashMap<>();
 
@@ -222,7 +226,7 @@ public class MojoExecutor
     private class ProjectLock implements AutoCloseable
     {
         final Lock acquiredAggregatorLock;
-        final Lock acquiredProjectLock;
+        final OwnerReentrantLock acquiredProjectLock;
 
         ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
         {
@@ -232,8 +236,31 @@ public class MojoExecutor
                 boolean aggregator = mojoDescriptor.isAggregator();
                 acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
                 acquiredProjectLock = getProjectLock( session );
-                acquiredAggregatorLock.lock();
-                acquiredProjectLock.lock();
+                if ( !acquiredAggregatorLock.tryLock() )
+                {
+                    Thread owner = aggregatorLock.getOwner();
+                    MojoDescriptor ownerMojo = owner != null ? mojos.get( owner ) : null;
+                    String str = ownerMojo != null ? " The " + ownerMojo.getId() : "An";
+                    String msg = str + " aggregator mojo is already being executed "
+                            + "in this parallel build, those kind of mojos require exclusive access to "
+                            + "reactor to prevent race conditions. This mojo execution will be blocked "
+                            + "until the aggregator mojo is done.";
+                    warn( msg );
+                    acquiredAggregatorLock.lock();
+                }
+                if ( !acquiredProjectLock.tryLock() )
+                {
+                    Thread owner = acquiredProjectLock.getOwner();
+                    MojoDescriptor ownerMojo = owner != null ? mojos.get( owner ) : null;
+                    String str = ownerMojo != null ? " The " + ownerMojo.getId() : "A";
+                    String msg = str + " mojo is already being executed "
+                            + "on the project " + session.getCurrentProject().getGroupId()
+                            + ":" + session.getCurrentProject().getArtifactId() + ". "
+                            + "This mojo execution will be blocked "
+                            + "until the mojo is done.";
+                    warn( msg );
+                    acquiredProjectLock.lock();
+                }
             }
             else
             {
@@ -258,10 +285,10 @@ public class MojoExecutor
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
-        private Lock getProjectLock( MavenSession session )
+        private OwnerReentrantLock getProjectLock( MavenSession session )
         {
             SessionData data = session.getRepositorySession().getData();
-            ConcurrentMap<MavenProject, Lock> locks = ( ConcurrentMap ) data.get( ProjectLock.class );
+            ConcurrentMap<MavenProject, OwnerReentrantLock> locks = ( ConcurrentMap ) data.get( ProjectLock.class );
             // initialize the value if not already done (in case of a concurrent access) to the method
             if ( locks == null )
             {
@@ -269,11 +296,11 @@ public class MojoExecutor
                 data.set( ProjectLock.class, null, new ConcurrentHashMap<>() );
                 locks = ( ConcurrentMap ) data.get( ProjectLock.class );
             }
-            Lock acquiredProjectLock = locks.get( session.getCurrentProject() );
+            OwnerReentrantLock acquiredProjectLock = locks.get( session.getCurrentProject() );
             if ( acquiredProjectLock == null )
             {
-                acquiredProjectLock = new ReentrantLock();
-                Lock prev = locks.putIfAbsent( session.getCurrentProject(), acquiredProjectLock );
+                acquiredProjectLock = new OwnerReentrantLock();
+                OwnerReentrantLock prev = locks.putIfAbsent( session.getCurrentProject(), acquiredProjectLock );
                 if ( prev != null )
                 {
                     acquiredProjectLock = prev;
@@ -283,6 +310,32 @@ public class MojoExecutor
         }
     }
 
+    static class OwnerReentrantLock extends ReentrantLock
+    {
+        @Override
+        public Thread getOwner()
+        {
+            return super.getOwner();
+        }
+    }
+
+    static class OwnerReentrantReadWriteLock extends ReentrantReadWriteLock
+    {
+        @Override
+        public Thread getOwner()
+        {
+            return super.getOwner();
+        }
+    }
+
+    private static void warn( String msg )
+    {
+        for ( String s : MultilineMessageHelper.format( msg ) )
+        {
+            LOGGER.warn( s );
+        }
+    }
+
     private void doExecute( MavenSession session, MojoExecution mojoExecution, ProjectIndex projectIndex,
                             DependencyContext dependencyContext )
             throws LifecycleExecutionException