You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2021/10/16 12:56:51 UTC
[maven] 01/01: [MNG-7312] Revert ThreadLocal approach from MNG-6843
and MNG-7251
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch MNG-7312
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 76dd2feb25ce8b3bc75d8d31f44bbbe41202d807
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sat Oct 16 14:32:24 2021 +0200
[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.
---
.../org/apache/maven/project/MavenProject.java | 87 +++++++++++-----------
.../org/apache/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 bbafcdf..57069a5 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<Artifact> resolvedArtifacts;
+ private ArtifactFilter artifactFilter;
+
+ private Set<Artifact> artifacts;
+
private Artifact parentArtifact;
private Set<Artifact> pluginArtifacts;
@@ -147,7 +151,8 @@ public class MavenProject
private Artifact artifact;
- private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new );
+ // calculated.
+ private Map<String, Artifact> artifactMap;
private Model originalModel;
@@ -690,11 +695,10 @@ public class MavenProject
public void setArtifacts( Set<Artifact> 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 class MavenProject
*/
public Set<Artifact> 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<String, Artifact> 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<Artifact> pluginArtifacts )
@@ -1176,8 +1178,7 @@ public class MavenProject
{
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 @@ public class MavenProject
// 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 @@ public class MavenProject
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 class MavenProject
*/
public void setResolvedArtifacts( Set<Artifact> artifacts )
{
- this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.emptySet();
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- artifactsHolder.artifacts = null;
- artifactsHolder.artifactMap = null;
+ this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.<Artifact>emptySet();
+ this.artifacts = null;
+ this.artifactMap = null;
}
/**
@@ -1438,10 +1442,9 @@ public class MavenProject
*/
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 class MavenProject
// ----------------------------------------------------------------------------------------------------------------
//
- // 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 class MavenProject
if ( moduleFile != null )
{
File moduleDir = moduleFile.getCanonicalFile().getParentFile();
+
module = moduleDir.getName();
}
@@ -1812,6 +1822,7 @@ public class MavenProject
public void setReportArtifacts( Set<Artifact> reportArtifacts )
{
this.reportArtifacts = reportArtifacts;
+
reportArtifactMap = null;
}
@@ -1828,6 +1839,7 @@ public class MavenProject
{
reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() );
}
+
return reportArtifactMap;
}
@@ -1835,6 +1847,7 @@ public class MavenProject
public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
{
this.extensionArtifacts = extensionArtifacts;
+
extensionArtifactMap = null;
}
@@ -1851,6 +1864,7 @@ public class MavenProject
{
extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() );
}
+
return extensionArtifactMap;
}
@@ -1868,6 +1882,7 @@ public class MavenProject
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 class MavenProject
{
this.projectBuilderConfiguration = projectBuildingRequest;
}
-
- private static class ArtifactsHolder
- {
- private ArtifactFilter artifactFilter;
- private Set<Artifact> artifacts;
- private Map<String, Artifact> 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 aff0dde..d2cba20 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 @@ package org.apache.maven.project;
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 class MavenProjectTest
}
@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<Set<Artifact>> 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
{