From 0024f62f61f16791edfce294c89e5c5860aa1ca8 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 2 Jun 2021 08:39:43 +0200 Subject: [PATCH 1/4] [MNG-7156] Parallel build can cause issues between clean and forked goals --- .../maven/execution/LockingEventSpy.java | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java diff --git a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java new file mode 100644 index 00000000000..5e412eb1c24 --- /dev/null +++ b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java @@ -0,0 +1,96 @@ +package org.apache.maven.execution; + +/* + * 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.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.apache.maven.eventspy.AbstractEventSpy; +import org.apache.maven.eventspy.EventSpy; +import org.apache.maven.project.MavenProject; +import org.eclipse.aether.SessionData; +import org.eclipse.sisu.Typed; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * EventSpy implementation that provides a per-project locking mechanism + * to make sure a given project can not be build twice concurrently. + * This case can happen when running parallel builds with forked lifecycles. + */ +@Singleton +@Named +@Typed( EventSpy.class ) +public class LockingEventSpy extends AbstractEventSpy +{ + + private final Logger logger = LoggerFactory.getLogger( getClass() ); + + private static final Object LOCKS_KEY = new Object(); + + @SuppressWarnings( { "unchecked", "rawtypes" } ) + private Lock getLock( ExecutionEvent event ) + { + SessionData data = event.getSession().getRepositorySession().getData(); + Map locks = ( Map ) data.get( LOCKS_KEY ); + if ( locks == null ) + { + data.set( LOCKS_KEY, null, new ConcurrentHashMap<>() ); + locks = ( Map ) data.get( LOCKS_KEY ); + } + return locks.computeIfAbsent( event.getProject(), p -> new ReentrantLock() ); + } + + @Override + public void onEvent( Object event ) throws Exception + { + if ( event instanceof ExecutionEvent ) + { + ExecutionEvent ee = ( ExecutionEvent ) event; + switch ( ee.getType() ) + { + case ProjectStarted: + case ForkedProjectStarted: + Lock lock = getLock( ee ); + if ( !lock.tryLock() ) + { + logger.warn( "Suspending concurrent execution of project " + ee.getProject() ); + getLock( ee ).lockInterruptibly(); + logger.warn( "Resuming execution of project " + ee.getProject() ); + } + break; + case ProjectSucceeded: + case ProjectFailed: + case ForkedProjectSucceeded: + case ForkedProjectFailed: + getLock( ee ).unlock(); + break; + default: + break; + } + } + } + +} From eefdf8d19ebc4b1ee28da31856ea7bfff7d43f86 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 2 Jun 2021 08:39:43 +0200 Subject: [PATCH 2/4] [MNG-7156] Parallel build can cause issues between clean and forked goals (code cleanup) --- .../maven/execution/LockingEventSpy.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java index 5e412eb1c24..20adf6f61d6 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java +++ b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java @@ -46,22 +46,9 @@ public class LockingEventSpy extends AbstractEventSpy { - private final Logger logger = LoggerFactory.getLogger( getClass() ); - private static final Object LOCKS_KEY = new Object(); - @SuppressWarnings( { "unchecked", "rawtypes" } ) - private Lock getLock( ExecutionEvent event ) - { - SessionData data = event.getSession().getRepositorySession().getData(); - Map locks = ( Map ) data.get( LOCKS_KEY ); - if ( locks == null ) - { - data.set( LOCKS_KEY, null, new ConcurrentHashMap<>() ); - locks = ( Map ) data.get( LOCKS_KEY ); - } - return locks.computeIfAbsent( event.getProject(), p -> new ReentrantLock() ); - } + private final Logger logger = LoggerFactory.getLogger( getClass() ); @Override public void onEvent( Object event ) throws Exception @@ -77,7 +64,7 @@ public void onEvent( Object event ) throws Exception if ( !lock.tryLock() ) { logger.warn( "Suspending concurrent execution of project " + ee.getProject() ); - getLock( ee ).lockInterruptibly(); + lock.lockInterruptibly(); logger.warn( "Resuming execution of project " + ee.getProject() ); } break; @@ -93,4 +80,17 @@ public void onEvent( Object event ) throws Exception } } + @SuppressWarnings( { "unchecked", "rawtypes" } ) + private Lock getLock( ExecutionEvent event ) + { + SessionData data = event.getSession().getRepositorySession().getData(); + Map locks = ( Map ) data.get( LOCKS_KEY ); + if ( locks == null ) + { + data.set( LOCKS_KEY, null, new ConcurrentHashMap<>() ); + locks = ( Map ) data.get( LOCKS_KEY ); + } + return locks.computeIfAbsent( event.getProject(), p -> new ReentrantLock() ); + } + } From 654b40c0b6cacc0258667df3f7ba1cad0317650a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 7 Jun 2021 09:04:51 +0200 Subject: [PATCH 3/4] [MNG-7156] Fixes after review --- .../apache/maven/execution/LockingEventSpy.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java index 20adf6f61d6..399e35fdf2c 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java +++ b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java @@ -37,7 +37,7 @@ /** * EventSpy implementation that provides a per-project locking mechanism - * to make sure a given project can not be build twice concurrently. + * to make sure a given project can not be built concurrently. * This case can happen when running parallel builds with forked lifecycles. */ @Singleton @@ -55,24 +55,24 @@ public void onEvent( Object event ) throws Exception { if ( event instanceof ExecutionEvent ) { - ExecutionEvent ee = ( ExecutionEvent ) event; - switch ( ee.getType() ) + ExecutionEvent executionEvent = ( ExecutionEvent ) event; + switch ( executionEvent.getType() ) { case ProjectStarted: case ForkedProjectStarted: - Lock lock = getLock( ee ); + Lock lock = getLock( executionEvent ); if ( !lock.tryLock() ) { - logger.warn( "Suspending concurrent execution of project " + ee.getProject() ); + logger.warn( "Suspending concurrent execution of project '{}'", executionEvent.getProject() ); lock.lockInterruptibly(); - logger.warn( "Resuming execution of project " + ee.getProject() ); + logger.warn( "Resuming execution of project '{}'", executionEvent.getProject() ); } break; case ProjectSucceeded: case ProjectFailed: case ForkedProjectSucceeded: case ForkedProjectFailed: - getLock( ee ).unlock(); + getLock( executionEvent ).unlock(); break; default: break; @@ -85,8 +85,10 @@ private Lock getLock( ExecutionEvent event ) { SessionData data = event.getSession().getRepositorySession().getData(); Map locks = ( Map ) data.get( LOCKS_KEY ); + // 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( LOCKS_KEY, null, new ConcurrentHashMap<>() ); locks = ( Map ) data.get( LOCKS_KEY ); } From f7b04e965e1b2aea81925775f62a9f89a48f368a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 12 Oct 2021 07:57:35 +0200 Subject: [PATCH 4/4] Tone down logging to debug when pausing concurrent execution --- .../main/java/org/apache/maven/execution/LockingEventSpy.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java index 399e35fdf2c..801e49bccc0 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java +++ b/maven-core/src/main/java/org/apache/maven/execution/LockingEventSpy.java @@ -63,9 +63,9 @@ public void onEvent( Object event ) throws Exception Lock lock = getLock( executionEvent ); if ( !lock.tryLock() ) { - logger.warn( "Suspending concurrent execution of project '{}'", executionEvent.getProject() ); + logger.debug( "Suspending concurrent execution of project '{}'", executionEvent.getProject() ); lock.lockInterruptibly(); - logger.warn( "Resuming execution of project '{}'", executionEvent.getProject() ); + logger.debug( "Resuming execution of project '{}'", executionEvent.getProject() ); } break; case ProjectSucceeded: