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:31:37 UTC

[maven] branch master updated (6767f2500 -> 7da0214d1)

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

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


    from 6767f2500 [MNG-7486] Create a multiline message helper for boxed log messages
     new 2a276d0dc [MNG-7487] Fix deadlock during forked lifecycle executions
     new 7da0214d1 [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     | 106 ++++++++++++++++-----
 1 file changed, 84 insertions(+), 22 deletions(-)


[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 master
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 7da0214d1d77a521ead7971f6c09209a80d1ddf3
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
    
    # Conflicts:
    #       maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java
---
 .../maven/lifecycle/internal/MojoExecutor.java     | 69 +++++++++++++++++++---
 1 file changed, 61 insertions(+), 8 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 bbd25389e..d251d5195 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
@@ -29,7 +29,6 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -43,6 +42,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;
@@ -59,6 +59,8 @@ import org.apache.maven.plugin.descriptor.MojoDescriptor;
 import org.apache.maven.project.MavenProject;
 import org.codehaus.plexus.util.StringUtils;
 import org.eclipse.aether.SessionData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * <p>
@@ -76,12 +78,14 @@ import org.eclipse.aether.SessionData;
 public class MojoExecutor
 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger( MojoExecutor.class );
+
     private final BuildPluginManager pluginManager;
     private final MavenPluginManager mavenPluginManager;
     private final LifecycleDependencyResolver lifeCycleDependencyResolver;
     private final ExecutionEventCatapult eventCatapult;
 
-    private final ReadWriteLock aggregatorLock = new ReentrantReadWriteLock();
+    private final OwnerReentrantReadWriteLock aggregatorLock = new OwnerReentrantReadWriteLock();
 
     private final Provider<MojosExecutionStrategy> mojosExecutionStrategy;
 
@@ -239,7 +243,7 @@ public class MojoExecutor
     private class ProjectLock implements AutoCloseable
     {
         final Lock acquiredAggregatorLock;
-        final Lock acquiredProjectLock;
+        final OwnerReentrantLock acquiredProjectLock;
 
         ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
         {
@@ -249,8 +253,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
             {
@@ -275,13 +302,13 @@ public class MojoExecutor
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
-        private Lock getProjectLock( MavenSession session )
+        private OwnerReentrantLock getProjectLock( MavenSession session )
         {
             SessionData data = session.getRepositorySession().getData();
             // TODO: when resolver 1.7.3 is released, the code below should be changed to
             // TODO: Map<MavenProject, Lock> locks = ( Map ) ((Map) data).computeIfAbsent(
             // TODO:         ProjectLock.class, l -> new ConcurrentHashMap<>() );
-            Map<MavenProject, Lock> locks = ( Map ) data.get( ProjectLock.class );
+            Map<MavenProject, OwnerReentrantLock> locks = ( Map ) data.get( ProjectLock.class );
             // initialize the value if not already done (in case of a concurrent access) to the method
             if ( locks == null )
             {
@@ -289,7 +316,33 @@ public class MojoExecutor
                 data.set( ProjectLock.class, null, new ConcurrentHashMap<>() );
                 locks = ( Map ) data.get( ProjectLock.class );
             }
-            return locks.computeIfAbsent( session.getCurrentProject(), p -> new ReentrantLock() );
+            return locks.computeIfAbsent( session.getCurrentProject(), p -> new OwnerReentrantLock() );
+        }
+    }
+
+    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 );
         }
     }
 


[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 master
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 2a276d0dcf5108a896eb44f2940baf98af2edbac
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
    
    # 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 6c089863c..bbd25389e 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 Provider<MojosExecutionStrategy> mojosExecutionStrategy;
 
+    private final Map<Thread, MojoDescriptor> mojos = new ConcurrentHashMap<>();
+
     @Inject
     public MojoExecutor(
             BuildPluginManager pluginManager,
@@ -223,10 +225,7 @@ public class MojoExecutor
             }
         }
 
-        try ( ProjectLock lock = new ProjectLock( session, mojoDescriptor, aggregatorLock ) )
-        {
-            doExecute( session, mojoExecution, projectIndex, dependencyContext );
-        }
+        doExecute( session, mojoExecution, projectIndex, dependencyContext );
     }
 
     /**
@@ -237,13 +236,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();
@@ -271,6 +271,7 @@ public class MojoExecutor
             {
                 acquiredAggregatorLock.unlock();
             }
+            mojos.remove( Thread.currentThread() );
         }
 
         @SuppressWarnings( { "unchecked", "rawtypes" } )
@@ -302,8 +303,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
@@ -324,13 +340,6 @@ public class MojoExecutor
 
             throw e;
         }
-        finally
-        {
-            for ( MavenProject forkedProject : forkedProjects )
-            {
-                forkedProject.setExecutionProject( null );
-            }
-        }
     }
 
     public void ensureDependenciesAreResolved( MojoDescriptor mojoDescriptor, MavenSession session,