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:13:23 UTC

[maven] branch maven-3.9.x updated (aec341a7a -> 0a94ff769)

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

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


    from aec341a7a [MNG-7486] Create a multiline message helper for boxed log messages
     new 0b0a96782 [MNG-7487] Fix deadlock during forked lifecycle executions
     new 0a94ff769 [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.9.x
in repository https://gitbox.apache.org/repos/asf/maven.git

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

    [MNG-7487] Fix deadlock during forked lifecycle executions
    
    # Conflicts:
    #       maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java
---
 .../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 2980e8858..408d09ac8 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
@@ -92,6 +92,8 @@ public class MojoExecutor
     @Requirement
     private PlexusContainer container;
 
+    private final Map<Thread, MojoDescriptor> mojos = new ConcurrentHashMap<>();
+
     public MojoExecutor()
     {
     }
@@ -228,10 +230,7 @@ public class MojoExecutor
             }
         }
 
-        try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor, aggregatorLock ) )
-        {
-            doExecute( session, mojoExecution, projectIndex, dependencyContext );
-        }
+        doExecute( session, mojoExecution, projectIndex, dependencyContext );
     }
 
     /**
@@ -242,13 +241,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();
@@ -276,6 +276,7 @@ public class MojoExecutor
             {
                 acquiredAggregatorLock.unlock();
             }
+            mojos.remove( Thread.currentThread() );
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
@@ -314,8 +315,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
@@ -336,13 +352,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.9.x
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 0a94ff769c6a3d2a8ec997353303407824374ca3
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 408d09ac8..c3117cf4f 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;
@@ -44,6 +45,8 @@ import org.codehaus.plexus.component.annotations.Requirement;
 import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
 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;
@@ -56,7 +59,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;
 
@@ -75,6 +77,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 public class MojoExecutor
 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger( MojoExecutor.class );
+
     @Requirement
     private BuildPluginManager pluginManager;
 
@@ -87,7 +91,7 @@ public class MojoExecutor
     @Requirement
     private ExecutionEventCatapult eventCatapult;
 
-    private final ReadWriteLock aggregatorLock = new ReentrantReadWriteLock();
+    private final OwnerReentrantReadWriteLock aggregatorLock = new OwnerReentrantReadWriteLock();
 
     @Requirement
     private PlexusContainer container;
@@ -244,7 +248,7 @@ public class MojoExecutor
     private class ProjectLock implements AutoCloseable
     {
         final Lock acquiredAggregatorLock;
-        final Lock acquiredProjectLock;
+        final OwnerReentrantLock acquiredProjectLock;
 
         ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
         {
@@ -254,8 +258,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
             {
@@ -280,10 +307,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 )
             {
@@ -291,11 +318,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;
@@ -305,6 +332,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