Skip to content

Commit

Permalink
[MNG-7476] Display a warning when an aggregator mojo is lock other mo…
Browse files Browse the repository at this point in the history
…jos executions
  • Loading branch information
gnodet committed May 17, 2022
1 parent 3686fec commit 3786e53
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package org.apache.maven.internal;

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import java.util.ArrayList;
import java.util.List;

/**
* Helper class to format warning messages to the console
*/
public class MessageHelper
{

private static final int DEFAULT_MAX_SIZE = 65;
private static final char BOX_CHAR = '*';

public static String separatorLine()
{
StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
repeat( sb, '*', DEFAULT_MAX_SIZE );
return sb.toString();
}

public static List<String> formatWarning( String... lines )
{
int size = DEFAULT_MAX_SIZE;
int rem = size - 4; // 4 chars = 2 box_char + 2 spaces
List<String> result = new ArrayList<>();
StringBuilder sb = new StringBuilder( size );
// first line
sb.setLength( 0 );
repeat( sb, BOX_CHAR, size );
result.add( sb.toString() );
// lines
for ( String line : lines )
{
sb.setLength( 0 );
String[] words = line.split( " " );
for ( String word : words )
{
if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
{
repeat( sb, ' ', rem - sb.length() );
result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
sb.setLength( 0 );
}
if ( sb.length() > 0 )
{
sb.append( ' ' );
}
sb.append( word );
}

while ( sb.length() < rem )
{
sb.append( ' ' );
}
result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
}
// last line
sb.setLength( 0 );
repeat( sb, BOX_CHAR, size );
result.add( sb.toString() );
return result;
}

private static void repeat( StringBuilder sb, char c, int nb )
{
for ( int i = 0; i < nb; i++ )
{
sb.append( c );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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.MessageHelper;
import org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.lifecycle.MissingProjectException;
import org.apache.maven.plugin.BuildPluginManager;
Expand All @@ -40,6 +41,8 @@
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;
Expand All @@ -52,7 +55,6 @@
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;

Expand All @@ -71,6 +73,8 @@
public class MojoExecutor
{

private static final Logger LOGGER = LoggerFactory.getLogger( MojoExecutor.class );

@Requirement
private BuildPluginManager pluginManager;

Expand All @@ -83,7 +87,9 @@ 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<>();

public MojoExecutor()
{
Expand Down Expand Up @@ -217,20 +223,44 @@ 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;
final OwnerReentrantLock 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();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredAggregatorLock = mojoDescriptor.isAggregator()
? aggregatorLock.writeLock() : aggregatorLock.readLock();
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();
}
acquiredProjectLock = getProjectLock( session );
acquiredAggregatorLock.lock();
acquiredProjectLock.lock();
if ( !acquiredProjectLock.tryLock() )
{
Thread owner = acquiredProjectLock.getOwner();
MojoDescriptor ownerMojo = owner != null ? mojos.get( owner ) : null;
String str = ownerMojo != null ? " The " + ownerMojo.getId() : "An ";
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
{
Expand All @@ -254,22 +284,22 @@ public void close()
}

@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 )
{
// the call to data.set(k, null, v) is effectively a call to data.putIfAbsent(k, v)
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;
Expand All @@ -279,6 +309,32 @@ private Lock getProjectLock( MavenSession session )
}
}

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 : MessageHelper.formatWarning( msg ) )
{
LOGGER.warn( s );
}
}

private void doExecute( MavenSession session, MojoExecution mojoExecution, ProjectIndex projectIndex,
DependencyContext dependencyContext )
throws LifecycleExecutionException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.maven.execution.ExecutionEvent;
import org.apache.maven.execution.MavenExecutionRequest;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.internal.MessageHelper;
import org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.lifecycle.LifecycleNotFoundException;
import org.apache.maven.lifecycle.LifecyclePhaseNotFoundException;
Expand Down Expand Up @@ -103,15 +104,15 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
final Set<Plugin> unsafePlugins = executionPlan.getNonThreadSafePlugins();
if ( !unsafePlugins.isEmpty() )
{
logger.warn( "*****************************************************************" );
logger.warn( "* Your build is requesting parallel execution, but project *" );
logger.warn( "* contains the following plugin(s) that have goals not marked *" );
logger.warn( "* as @threadSafe to support parallel building. *" );
logger.warn( "* While this /may/ work fine, please look for plugin updates *" );
logger.warn( "* and/or request plugins be made thread-safe. *" );
logger.warn( "* If reporting an issue, report it against the plugin in *" );
logger.warn( "* question, not against maven-core *" );
logger.warn( "*****************************************************************" );
for ( String s : MessageHelper.formatWarning(
"Your build is requesting parallel execution, but project contains the following "
+ "plugin(s) that have goals not marked as @threadSafe to support parallel building.",
"While this /may/ work fine, please look for plugin updates and/or "
+ "request plugins be made thread-safe.",
"If reporting an issue, report it against the plugin in question, not against maven-core." ) )
{
logger.warn( s );
}
if ( logger.isDebugEnabled() )
{
final Set<MojoDescriptor> unsafeGoals = executionPlan.getNonThreadSafeMojos();
Expand All @@ -130,7 +131,7 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
}
logger.warn( "Enable debug to see more precisely which goals are not marked @threadSafe." );
}
logger.warn( "*****************************************************************" );
logger.warn( MessageHelper.separatorLine() );
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.apache.maven.internal;

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import java.util.ArrayList;
import java.util.List;

import org.junit.Test;

import static org.junit.Assert.assertEquals;

public class MessageHelperTest
{

@Test
public void testBuilderCommon()
{
List<String> msgs = new ArrayList<>();
msgs.add( "*****************************************************************" );
msgs.add( "* Your build is requesting parallel execution, but project *" );
msgs.add( "* contains the following plugin(s) that have goals not marked *" );
msgs.add( "* as @threadSafe to support parallel building. *" );
msgs.add( "* While this /may/ work fine, please look for plugin updates *" );
msgs.add( "* and/or request plugins be made thread-safe. *" );
msgs.add( "* If reporting an issue, report it against the plugin in *" );
msgs.add( "* question, not against maven-core *" );
msgs.add( "*****************************************************************" );

assertEquals( msgs, MessageHelper.formatWarning(
"Your build is requesting parallel execution, but project contains the following "
+ "plugin(s) that have goals not marked as @threadSafe to support parallel building.",
"While this /may/ work fine, please look for plugin updates and/or "
+ "request plugins be made thread-safe.",
"If reporting an issue, report it against the plugin in question, not against maven-core"
) );
}

@Test
public void testMojoExecutor()
{
List<String> msgs = new ArrayList<>();
msgs.add( "*****************************************************************" );
msgs.add( "* An aggregator Mojo is already executing in parallel build, *" );
msgs.add( "* but aggregator Mojos require exclusive access to reactor to *" );
msgs.add( "* prevent race conditions. This mojo execution will be blocked *" );
msgs.add( "* until the aggregator work is done. *" );
msgs.add( "*****************************************************************" );

assertEquals( msgs, MessageHelper.formatWarning(
"An aggregator Mojo is already executing in parallel build, but aggregator "
+ "Mojos require exclusive access to reactor to prevent race conditions. This "
+ "mojo execution will be blocked until the aggregator work is done." ) );
}
}

0 comments on commit 3786e53

Please sign in to comment.