From 8784812cf6af6c72597a8f3de45288ce7b1206a6 Mon Sep 17 00:00:00 2001 From: Michael Osipov Date: Sat, 16 Oct 2021 14:32:24 +0200 Subject: [PATCH] [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251 Revert "[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project" This reverts commit a6e462b53a4b6c87ef43f6cfe7fbde0b0939e3d6. Revert "[MNG-6843] Parallel build fails due to missing JAR artifacts in compilePath" This reverts commit 73e00ed85df84ba0c557dd020740812b2453f2d3. === This closes #594 --- .../apache/maven/project/MavenProject.java | 87 +++++++++---------- .../maven/project/MavenProjectTest.java | 43 --------- 2 files changed, 43 insertions(+), 87 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java index bbafcdf2b1..57069a5e6a 100644 --- a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java +++ b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java @@ -111,6 +111,10 @@ public class MavenProject private Set resolvedArtifacts; + private ArtifactFilter artifactFilter; + + private Set artifacts; + private Artifact parentArtifact; private Set pluginArtifacts; @@ -147,7 +151,8 @@ public class MavenProject private Artifact artifact; - private ThreadLocal threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new ); + // calculated. + private Map artifactMap; private Model originalModel; @@ -690,11 +695,10 @@ public void addLicense( License license ) public void setArtifacts( Set artifacts ) { - ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get(); - artifactsHolder.artifacts = artifacts; + this.artifacts = artifacts; // flush the calculated artifactMap - artifactsHolder.artifactMap = null; + artifactMap = null; } /** @@ -707,36 +711,34 @@ public void setArtifacts( Set artifacts ) */ public Set getArtifacts() { - ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get(); - if ( artifactsHolder.artifacts == null ) + if ( artifacts == null ) { - if ( artifactsHolder.artifactFilter == null || resolvedArtifacts == null ) + if ( artifactFilter == null || resolvedArtifacts == null ) { - artifactsHolder.artifacts = new LinkedHashSet<>(); + artifacts = new LinkedHashSet<>(); } else { - artifactsHolder.artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 ); + artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 ); for ( Artifact artifact : resolvedArtifacts ) { - if ( artifactsHolder.artifactFilter.include( artifact ) ) + if ( artifactFilter.include( artifact ) ) { - artifactsHolder.artifacts.add( artifact ); + artifacts.add( artifact ); } } } } - return artifactsHolder.artifacts; + return artifacts; } public Map getArtifactMap() { - ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get(); - if ( artifactsHolder.artifactMap == null ) + if ( artifactMap == null ) { - artifactsHolder.artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() ); + artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() ); } - return artifactsHolder.artifactMap; + return artifactMap; } public void setPluginArtifacts( Set pluginArtifacts ) @@ -1176,8 +1178,7 @@ public MavenProject clone() { throw new UnsupportedOperationException( e ); } - // clone must have its own TL, otherwise the artifacts are intermingled! - clone.threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new ); + clone.deepCopy( this ); return clone; @@ -1220,7 +1221,6 @@ private void deepCopy( MavenProject project ) // copy fields file = project.file; basedir = project.basedir; - threadLocalArtifactsHolder.set( project.threadLocalArtifactsHolder.get().copy() ); // don't need a deep copy, they don't get modified or added/removed to/from - but make them unmodifiable to be // sure! @@ -1229,6 +1229,11 @@ private void deepCopy( MavenProject project ) setDependencyArtifacts( Collections.unmodifiableSet( project.getDependencyArtifacts() ) ); } + if ( project.getArtifacts() != null ) + { + setArtifacts( Collections.unmodifiableSet( project.getArtifacts() ) ); + } + if ( project.getParentFile() != null ) { parentFile = new File( project.getParentFile().getAbsolutePath() ); @@ -1422,10 +1427,9 @@ public DependencyFilter getExtensionDependencyFilter() */ public void setResolvedArtifacts( Set artifacts ) { - this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.emptySet(); - ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get(); - artifactsHolder.artifacts = null; - artifactsHolder.artifactMap = null; + this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.emptySet(); + this.artifacts = null; + this.artifactMap = null; } /** @@ -1438,10 +1442,9 @@ public void setResolvedArtifacts( Set artifacts ) */ public void setArtifactFilter( ArtifactFilter artifactFilter ) { - ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get(); - artifactsHolder.artifactFilter = artifactFilter; - artifactsHolder.artifacts = null; - artifactsHolder.artifactMap = null; + this.artifactFilter = artifactFilter; + this.artifacts = null; + this.artifactMap = null; } /** @@ -1471,7 +1474,13 @@ public void addLifecyclePhase( String lifecyclePhase ) // ---------------------------------------------------------------------------------------------------------------- // - // D E P R E C A T E D - Everything below will be removed for Maven 4.0.0 + // + // D E P R E C A T E D + // + // + // ---------------------------------------------------------------------------------------------------------------- + // + // Everything below will be removed for Maven 4.0.0 // // ---------------------------------------------------------------------------------------------------------------- @@ -1492,6 +1501,7 @@ public String getModulePathAdjustment( MavenProject moduleProject ) if ( moduleFile != null ) { File moduleDir = moduleFile.getCanonicalFile().getParentFile(); + module = moduleDir.getName(); } @@ -1812,6 +1822,7 @@ public Reporting getReporting() public void setReportArtifacts( Set reportArtifacts ) { this.reportArtifacts = reportArtifacts; + reportArtifactMap = null; } @@ -1828,6 +1839,7 @@ public Map getReportArtifactMap() { reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() ); } + return reportArtifactMap; } @@ -1835,6 +1847,7 @@ public Map getReportArtifactMap() public void setExtensionArtifacts( Set extensionArtifacts ) { this.extensionArtifacts = extensionArtifacts; + extensionArtifactMap = null; } @@ -1851,6 +1864,7 @@ public Map getExtensionArtifactMap() { extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() ); } + return extensionArtifactMap; } @@ -1868,6 +1882,7 @@ public List getReportPlugins() public Xpp3Dom getReportConfiguration( String pluginGroupId, String pluginArtifactId, String reportSetId ) { Xpp3Dom dom = null; + // ---------------------------------------------------------------------- // I would like to be able to lookup the Mojo object using a key but // we have a limitation in modello that will be remedied shortly. So @@ -1971,20 +1986,4 @@ public void setProjectBuildingRequest( ProjectBuildingRequest projectBuildingReq { this.projectBuilderConfiguration = projectBuildingRequest; } - - private static class ArtifactsHolder - { - private ArtifactFilter artifactFilter; - private Set artifacts; - private Map artifactMap; - - ArtifactsHolder copy() - { - ArtifactsHolder copy = new ArtifactsHolder(); - copy.artifactFilter = artifactFilter; - copy.artifacts = artifacts != null ? new LinkedHashSet<>( artifacts ) : null; - copy.artifactMap = artifactMap != null ? new LinkedHashMap<>( artifactMap ) : null; - return copy; - } - } } diff --git a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java index aff0dde386..d2cba20a41 100644 --- a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java +++ b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java @@ -21,19 +21,14 @@ import java.io.File; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; -import org.apache.maven.artifact.Artifact; import org.apache.maven.model.DependencyManagement; import org.apache.maven.model.Model; import org.apache.maven.model.Parent; import org.apache.maven.model.Profile; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -209,44 +204,6 @@ public void testCloneWithBaseDir() } @Test - public void testCloneWithArtifacts() - throws InterruptedException - { - Artifact initialArtifact = Mockito.mock( Artifact.class, "initialArtifact" ); - MavenProject originalProject = new MavenProject(); - originalProject.setArtifacts( Collections.singleton( initialArtifact ) ); - assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(), - "Sanity check: originalProject returns artifact that has just been set" ); - - final MavenProject clonedProject = originalProject.clone(); - - assertEquals( Collections.singleton( initialArtifact ), clonedProject.getArtifacts(), - "Cloned project returns the artifact that was set for the original project" ); - - Artifact anotherArtifact = Mockito.mock( Artifact.class, "anotherArtifact" ); - clonedProject.setArtifacts( Collections.singleton( anotherArtifact ) ); - assertEquals( Collections.singleton( anotherArtifact ), clonedProject.getArtifacts(), - "Sanity check: clonedProject returns artifact that has just been set" ); - - assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(), - "Original project returns the artifact that was set initially (not the one for clonedProject)" ); - - final AtomicReference> artifactsFromThread = new AtomicReference<>(); - Thread thread = new Thread( new Runnable() - { - @Override - public void run() - { - artifactsFromThread.set( clonedProject.getArtifacts() ); - } - } ); - thread.start(); - thread.join(); - - assertEquals( Collections.emptySet(), artifactsFromThread.get(), - "Another thread does not see the same artifacts" ); - } - public void testUndefinedOutputDirectory() throws Exception {