diff --git a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java index 98c5573c97..5e6f1b99f2 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java +++ b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java @@ -54,7 +54,7 @@ public class MavenSession private Properties executionProperties; - private MavenProject currentProject; + private ThreadLocal currentProject = new ThreadLocal<>(); /** * These projects have already been topologically sorted in the {@link org.apache.maven.Maven} component before @@ -83,14 +83,15 @@ public class MavenSession { if ( !projects.isEmpty() ) { - this.currentProject = projects.get( 0 ); + MavenProject first = projects.get( 0 ); + this.currentProject = ThreadLocal.withInitial( () -> first ); this.topLevelProject = projects.stream().filter( project -> project.isExecutionRoot() ).findFirst() - .orElse( currentProject ); + .orElse( first ); } else { - this.currentProject = null; + this.currentProject = new ThreadLocal<>(); this.topLevelProject = null; } this.projects = projects; @@ -151,12 +152,12 @@ public class MavenSession public void setCurrentProject( MavenProject currentProject ) { - this.currentProject = currentProject; + this.currentProject.set( currentProject ); } public MavenProject getCurrentProject() { - return currentProject; + return currentProject.get(); } public ProjectBuildingRequest getProjectBuildingRequest() @@ -233,7 +234,12 @@ public class MavenSession { try { - return (MavenSession) super.clone(); + MavenSession clone = (MavenSession) super.clone(); + // the default must become the current project of the thread that clones this + MavenProject current = getCurrentProject(); + // we replace the thread local of the clone to prevent write through and enforce the new default value + clone.currentProject = ThreadLocal.withInitial( () -> current ); + return clone; } catch ( CloneNotSupportedException e ) { diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java index 4fef69b4a9..2cceb3cde2 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/BuildListCalculator.java @@ -58,15 +58,16 @@ public class BuildListCalculator for ( MavenProject project : projects ) { ClassLoader tccl = Thread.currentThread().getContextClassLoader(); + MavenProject currentProject = session.getCurrentProject(); try { BuilderCommon.attachToThread( project ); // Not totally sure if this is needed for anything - MavenSession copiedSession = session.clone(); - copiedSession.setCurrentProject( project ); - projectBuilds.add( new ProjectSegment( project, taskSegment, copiedSession ) ); + session.setCurrentProject( project ); + projectBuilds.add( new ProjectSegment( project, taskSegment, session ) ); } finally { + session.setCurrentProject( currentProject ); Thread.currentThread().setContextClassLoader( tccl ); } } diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java index 548fe6c8ff..8fbba4da95 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java @@ -32,7 +32,6 @@ import org.apache.maven.lifecycle.MavenExecutionPlan; import org.apache.maven.lifecycle.internal.builder.BuilderCommon; import org.apache.maven.plugin.MojoExecution; import org.apache.maven.project.MavenProject; -import org.apache.maven.session.scope.internal.SessionScope; import org.codehaus.plexus.component.annotations.Component; import org.codehaus.plexus.component.annotations.Requirement; @@ -66,9 +65,6 @@ public class LifecycleModuleBuilder @Requirement private List projectExecutionListeners; - @Requirement - private SessionScope sessionScope; - public void setProjectExecutionListeners( final List listeners ) { this.projectExecutionListeners = listeners; @@ -88,10 +84,6 @@ public class LifecycleModuleBuilder long buildStartTime = System.currentTimeMillis(); - // session may be different from rootSession seeded in DefaultMaven - // explicitly seed the right session here to make sure it is used by Guice - sessionScope.enter( reactorContext.getSessionScopeMemento() ); - sessionScope.seed( MavenSession.class, session ); try { @@ -145,8 +137,6 @@ public class LifecycleModuleBuilder } finally { - sessionScope.exit(); - session.setCurrentProject( null ); Thread.currentThread().setContextClassLoader( reactorContext.getOriginalContextClassLoader() ); diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java index 9bd74328b6..794a0718a7 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java @@ -106,9 +106,7 @@ public class LifecycleStarter ClassLoader oldContextClassLoader = Thread.currentThread().getContextClassLoader(); ReactorBuildStatus reactorBuildStatus = new ReactorBuildStatus( session.getProjectDependencyGraph() ); - reactorContext = - new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus, - sessionScope.memento() ); + reactorContext = new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus ); String builderId = session.getRequest().getBuilderId(); Builder builder = builders.get( builderId ); diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java index 7df5314045..076c6229f8 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java @@ -20,7 +20,6 @@ package org.apache.maven.lifecycle.internal; */ import org.apache.maven.execution.MavenExecutionResult; -import org.apache.maven.session.scope.internal.SessionScope; /** * Context that is fixed for the entire reactor build. @@ -40,17 +39,13 @@ public class ReactorContext private final ReactorBuildStatus reactorBuildStatus; - private final SessionScope.Memento sessionScope; - public ReactorContext( MavenExecutionResult result, ProjectIndex projectIndex, - ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus, - SessionScope.Memento sessionScope ) + ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus ) { this.result = result; this.projectIndex = projectIndex; this.originalContextClassLoader = originalContextClassLoader; this.reactorBuildStatus = reactorBuildStatus; - this.sessionScope = sessionScope; } public ReactorBuildStatus getReactorBuildStatus() @@ -73,11 +68,4 @@ public class ReactorContext return originalContextClassLoader; } - /** - * @since 3.3.0 - */ - public SessionScope.Memento getSessionScopeMemento() - { - return sessionScope; - } } diff --git a/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java b/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java index ac423bc6c9..7bbf477988 100644 --- a/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java +++ b/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java @@ -19,16 +19,16 @@ package org.apache.maven.session.scope.internal; * under the License. */ -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedList; +import java.util.Collection; +import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import com.google.inject.Key; import com.google.inject.OutOfScopeException; import com.google.inject.Provider; import com.google.inject.Scope; -import com.google.inject.util.Providers; /** * SessionScope @@ -36,134 +36,112 @@ import com.google.inject.util.Providers; public class SessionScope implements Scope { - /** - * @since 3.3.0 - */ - public static class Memento - { - final Map, Provider> seeded; - Memento( final Map, Provider> seeded ) - { - this.seeded = Collections.unmodifiableMap( new HashMap<>( seeded ) ); - } - } - - private static final Provider SEEDED_KEY_PROVIDER = new Provider() + private static final Provider SEEDED_KEY_PROVIDER = () -> { - public Object get() - { - throw new IllegalStateException(); - } + throw new IllegalStateException(); }; /** * ScopeState */ - private static final class ScopeState + protected static final class ScopeState { - private final Map, Provider> seeded = new HashMap<>(); + private final Map, CachingProvider> provided = new ConcurrentHashMap<>(); - private final Map, Object> provided = new HashMap<>(); + public void seed( Class clazz, Provider value ) + { + provided.put( Key.get( clazz ), new CachingProvider<>( value ) ); + } + + @SuppressWarnings( "unchecked" ) + public Provider scope( Key key, Provider unscoped ) + { + Provider provider = provided.computeIfAbsent( key, k -> new CachingProvider<>( unscoped ) ); + return ( Provider ) provider; + } + + public Collection> providers() + { + return provided.values(); + } } - private final ThreadLocal> values = new ThreadLocal<>(); + private final List values = new CopyOnWriteArrayList<>(); public void enter() { - LinkedList stack = values.get(); - if ( stack == null ) - { - stack = new LinkedList<>(); - values.set( stack ); - } - stack.addFirst( new ScopeState() ); + values.add( 0, new ScopeState() ); } - /** - * @since 3.3.0 - */ - public void enter( Memento memento ) + protected ScopeState getScopeState() { - enter(); - getScopeState().seeded.putAll( memento.seeded ); - } - - private ScopeState getScopeState() - { - LinkedList stack = values.get(); - if ( stack == null || stack.isEmpty() ) + if ( values.isEmpty() ) { - throw new IllegalStateException(); + throw new OutOfScopeException( "Cannot access session scope outside of a scoping block" ); } - return stack.getFirst(); + return values.get( 0 ); } public void exit() { - final LinkedList stack = values.get(); - if ( stack == null || stack.isEmpty() ) + if ( values.isEmpty() ) { throw new IllegalStateException(); } - stack.removeFirst(); - if ( stack.isEmpty() ) - { - values.remove(); - } - } - - /** - * @since 3.3.0 - */ - public Memento memento() - { - LinkedList stack = values.get(); - return new Memento( stack != null ? stack.getFirst().seeded : Collections., Provider>emptyMap() ); + values.remove( 0 ); } public void seed( Class clazz, Provider value ) { - getScopeState().seeded.put( Key.get( clazz ), value ); + getScopeState().seed( clazz, value ); } public void seed( Class clazz, final T value ) { - getScopeState().seeded.put( Key.get( clazz ), Providers.of( value ) ); + seed( clazz, ( Provider ) () -> value ); } public Provider scope( final Key key, final Provider unscoped ) { - return new Provider() + // Lazy evaluating provider + return () -> getScopeState().scope( key, unscoped ).get(); + } + + /** + * A provider wrapping an existing provider with a cache + * @param the provided type + */ + protected static class CachingProvider implements Provider + { + private final Provider provider; + private volatile T value; + + CachingProvider( Provider provider ) { - @SuppressWarnings( "unchecked" ) - public T get() + this.provider = provider; + } + + public T value() + { + return value; + } + + @Override + public T get() + { + if ( value == null ) { - LinkedList stack = values.get(); - if ( stack == null || stack.isEmpty() ) + synchronized ( this ) { - throw new OutOfScopeException( "Cannot access " + key + " outside of a scoping block" ); + if ( value == null ) + { + value = provider.get(); + } } - - ScopeState state = stack.getFirst(); - - Provider seeded = state.seeded.get( key ); - - if ( seeded != null ) - { - return (T) seeded.get(); - } - - T provided = (T) state.provided.get( key ); - if ( provided == null && unscoped != null ) - { - provided = unscoped.get(); - state.provided.put( key, provided ); - } - - return provided; } - }; + return value; + } } @SuppressWarnings( { "unchecked" } ) diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/BuildListCalculatorTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/BuildListCalculatorTest.java index a0039832af..eb36c52c70 100644 --- a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/BuildListCalculatorTest.java +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/BuildListCalculatorTest.java @@ -39,6 +39,11 @@ public class BuildListCalculatorTest assertEquals( "Stub data contains 6 items", 6, segments.size() ); final ProjectSegment build = segments.get( 0 ); assertNotNull( build ); + + for ( ProjectSegment segment : segments ) + { + assertSame( session, segment.getSession() ); + } } private static LifecycleTaskSegmentCalculator getTaskSegmentCalculator() diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilderTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilderTest.java new file mode 100644 index 0000000000..594af05b7f --- /dev/null +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilderTest.java @@ -0,0 +1,101 @@ +package org.apache.maven.lifecycle.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.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.maven.execution.AbstractExecutionListener; +import org.apache.maven.execution.DefaultMavenExecutionRequest; +import org.apache.maven.execution.DefaultMavenExecutionResult; +import org.apache.maven.execution.MavenExecutionRequest; +import org.apache.maven.execution.MavenExecutionResult; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.lifecycle.LifecycleExecutionException; +import org.apache.maven.lifecycle.internal.builder.BuilderCommon; +import org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder; +import org.apache.maven.lifecycle.internal.stub.DefaultLifecyclesStub; +import org.apache.maven.lifecycle.internal.stub.LifecycleTaskSegmentCalculatorStub; +import org.apache.maven.lifecycle.internal.stub.LoggerStub; +import org.apache.maven.lifecycle.internal.stub.MojoExecutorStub; +import org.apache.maven.lifecycle.internal.stub.ProjectDependencyGraphStub; +import org.apache.maven.plugin.MojoExecution; +import org.apache.maven.project.MavenProject; +import org.codehaus.plexus.ContainerConfiguration; +import org.codehaus.plexus.PlexusConstants; +import org.codehaus.plexus.PlexusTestCase; +import org.codehaus.plexus.logging.Logger; +import org.junit.Test; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +public class LifecycleModuleBuilderTest extends PlexusTestCase +{ + @Override + protected void customizeContainerConfiguration( ContainerConfiguration configuration ) + { + configuration.setAutoWiring( true ); + configuration.setClassPathScanning( PlexusConstants.SCANNING_INDEX ); + + } + + public void testCurrentProject() throws Exception + { + List currentProjects = new ArrayList<>(); + MojoExecutorStub mojoExecutor = new MojoExecutorStub() + { + @Override + public void execute( MavenSession session, List mojoExecutions, ProjectIndex projectIndex ) + throws LifecycleExecutionException + { + super.execute( session, mojoExecutions, projectIndex ); + currentProjects.add( session.getCurrentProject() ); + } + }; + + final DefaultMavenExecutionResult defaultMavenExecutionResult = new DefaultMavenExecutionResult(); + MavenExecutionRequest mavenExecutionRequest = new DefaultMavenExecutionRequest(); + mavenExecutionRequest.setExecutionListener( new AbstractExecutionListener() ); + mavenExecutionRequest.setGoals( Arrays.asList( "clean" ) ); + final MavenSession session = new MavenSession( null, null, mavenExecutionRequest, defaultMavenExecutionResult ); + final ProjectDependencyGraphStub dependencyGraphStub = new ProjectDependencyGraphStub(); + session.setProjectDependencyGraph( dependencyGraphStub ); + session.setProjects( dependencyGraphStub.getSortedProjects() ); + + LifecycleModuleBuilder moduleBuilder = lookup( LifecycleModuleBuilder.class ); + set( moduleBuilder, "mojoExecutor", mojoExecutor ); + + LifecycleStarter ls = lookup( LifecycleStarter.class ); + ls.execute( session ); + + assertNull( session.getCurrentProject() ); + assertEquals( Arrays.asList( ProjectDependencyGraphStub.A, ProjectDependencyGraphStub.B, ProjectDependencyGraphStub.C, + ProjectDependencyGraphStub.X, ProjectDependencyGraphStub.Y, ProjectDependencyGraphStub.Z ), currentProjects ); + } + + static void set( Object obj, String field, Object v ) throws NoSuchFieldException, IllegalAccessException + { + Field f = obj.getClass().getDeclaredField( field ); + f.setAccessible( true ); + f.set( obj, v ); + } + +}