Apache
/
maven
mirror of https://github.com/apache/maven.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251 

Revert "[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project"

This reverts commit a6e462b53a.

Revert "[MNG-6843] Parallel build fails due to missing JAR artifacts in compilePath"

This reverts commit 73e00ed85d.

===

This closes #594
This commit is contained in:
Michael Osipov 2021-10-16 14:32:24 +02:00
parent 965aaa53da
commit 8784812cf6
2 changed files with 43 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
maven-core/src/main/java/org/apache/maven/project/MavenProject.java
View File

@ -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;
}
}
}

43
maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
View File

@ -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
{