You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sc...@apache.org on 2017/01/31 03:02:27 UTC
[28/50] maven git commit: [MNG-6105]
properties.internal.SystemProperties.addSystemProperties() is not really
thread-safe
[MNG-6105] properties.internal.SystemProperties.addSystemProperties() is not really thread-safe
Refactoring the current code setting system properties to synchronize correctly on the given ones: avoids ConcurrentModificationException and NullPointerException if the properties is modified by another thread.
Project: http://git-wip-us.apache.org/repos/asf/maven/repo
Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/5b4b8bd9
Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/5b4b8bd9
Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/5b4b8bd9
Branch: refs/heads/MNG-5359-IT
Commit: 5b4b8bd94c87afd2a1527d6a860e9673bdaf4a22
Parents: 1e2a80e
Author: Guillaume Bou� <gb...@apache.org>
Authored: Sun Oct 16 01:40:46 2016 +0200
Committer: Guillaume Bou� <gb...@apache.org>
Committed: Sat Jan 28 14:07:59 2017 +0100
----------------------------------------------------------------------
.../internal/MavenRepositorySystemUtils.java | 14 +++++----
.../execution/DefaultMavenExecutionRequest.java | 4 +--
.../project/DefaultProjectBuildingRequest.java | 7 ++---
.../properties/internal/SystemProperties.java | 30 ++++++++++++++------
.../building/DefaultModelBuildingRequest.java | 5 +++-
.../DefaultSettingsBuildingRequest.java | 7 ++---
6 files changed, 42 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
----------------------------------------------------------------------
diff --git a/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java b/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
index 5b240ef..1b11cb3 100644
--- a/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
+++ b/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
@@ -126,14 +126,18 @@ public final class MavenRepositorySystemUtils
session.setArtifactDescriptorPolicy( new SimpleArtifactDescriptorPolicy( true, true ) );
+ final Properties systemProperties = new Properties();
+
// MNG-5670 guard against ConcurrentModificationException
- Properties sysProps = new Properties();
- for ( String key : System.getProperties().stringPropertyNames() )
+ // MNG-6053 guard against key without value
+ Properties sysProp = System.getProperties();
+ synchronized ( sysProp )
{
- sysProps.put( key, System.getProperty( key ) );
+ systemProperties.putAll( sysProp );
}
- session.setSystemProperties( sysProps );
- session.setConfigProperties( sysProps );
+
+ session.setSystemProperties( systemProperties );
+ session.setConfigProperties( systemProperties );
return session;
}
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
----------------------------------------------------------------------
diff --git a/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java b/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
index 71a6894..d67061f 100644
--- a/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
+++ b/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
@@ -33,6 +33,7 @@ import org.apache.maven.eventspy.internal.EventSpyDispatcher;
import org.apache.maven.model.Profile;
import org.apache.maven.project.DefaultProjectBuildingRequest;
import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.properties.internal.SystemProperties;
import org.apache.maven.settings.Mirror;
import org.apache.maven.settings.Proxy;
import org.apache.maven.settings.Server;
@@ -535,8 +536,7 @@ public class DefaultMavenExecutionRequest
{
if ( properties != null )
{
- this.systemProperties = new Properties();
- this.systemProperties.putAll( properties );
+ this.systemProperties = SystemProperties.copyProperties( properties );
}
else
{
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingRequest.java
----------------------------------------------------------------------
diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingRequest.java b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingRequest.java
index f439240..f1b271b 100644
--- a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingRequest.java
+++ b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingRequest.java
@@ -28,6 +28,7 @@ import org.apache.commons.lang3.Validate;
import org.apache.maven.artifact.repository.ArtifactRepository;
import org.apache.maven.model.Profile;
import org.apache.maven.model.building.ModelBuildingRequest;
+import org.apache.maven.properties.internal.SystemProperties;
import org.eclipse.aether.RepositorySystemSession;
public class DefaultProjectBuildingRequest
@@ -165,11 +166,7 @@ public class DefaultProjectBuildingRequest
{
if ( systemProperties != null )
{
- this.systemProperties = new Properties();
- synchronized ( systemProperties )
- { // avoid concurrentmodification if someone else sets/removes an unrelated system property
- this.systemProperties.putAll( systemProperties );
- }
+ this.systemProperties = SystemProperties.copyProperties( systemProperties );
}
else
{
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-core/src/main/java/org/apache/maven/properties/internal/SystemProperties.java
----------------------------------------------------------------------
diff --git a/maven-core/src/main/java/org/apache/maven/properties/internal/SystemProperties.java b/maven-core/src/main/java/org/apache/maven/properties/internal/SystemProperties.java
index aa5fed9..3d6a969 100644
--- a/maven-core/src/main/java/org/apache/maven/properties/internal/SystemProperties.java
+++ b/maven-core/src/main/java/org/apache/maven/properties/internal/SystemProperties.java
@@ -33,19 +33,33 @@ public class SystemProperties
*/
public static void addSystemProperties( Properties props )
{
- for ( String key : System.getProperties().stringPropertyNames() )
- {
- props.put( key, System.getProperty( key ) );
- }
+ props.putAll( getSystemProperties() );
}
/**
- * Returns System.properties copy.
+ * Returns a copy of {@link System#getProperties()} in a thread-safe manner.
+ *
+ * @return {@link System#getProperties()} obtained in a thread-safe manner.
*/
public static Properties getSystemProperties()
{
- Properties systemProperties = new Properties();
- addSystemProperties( systemProperties );
- return systemProperties;
+ return copyProperties( System.getProperties() );
+ }
+
+ /**
+ * Copies the given {@link Properties} object into a new {@link Properties} object, in a thread-safe manner.
+ * @param properties Properties to copy.
+ * @return Copy of the given properties.
+ */
+ public static Properties copyProperties( Properties properties )
+ {
+ final Properties copyProperties = new Properties();
+ // guard against modification/removal of keys in the given properties (MNG-5670, MNG-6053, MNG-6105)
+ synchronized ( properties )
+ {
+ copyProperties.putAll( properties );
+ }
+ return copyProperties;
}
+
}
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
----------------------------------------------------------------------
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
index a3505c9..84a68f7 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
@@ -286,7 +286,10 @@ public class DefaultModelBuildingRequest
if ( systemProperties != null )
{
this.systemProperties = new Properties();
- this.systemProperties.putAll( systemProperties );
+ synchronized ( systemProperties )
+ { // avoid concurrentmodification if someone else sets/removes an unrelated system property
+ this.systemProperties.putAll( systemProperties );
+ }
}
else
{
http://git-wip-us.apache.org/repos/asf/maven/blob/5b4b8bd9/maven-settings-builder/src/main/java/org/apache/maven/settings/building/DefaultSettingsBuildingRequest.java
----------------------------------------------------------------------
diff --git a/maven-settings-builder/src/main/java/org/apache/maven/settings/building/DefaultSettingsBuildingRequest.java b/maven-settings-builder/src/main/java/org/apache/maven/settings/building/DefaultSettingsBuildingRequest.java
index 5a4824e..4bb691b 100644
--- a/maven-settings-builder/src/main/java/org/apache/maven/settings/building/DefaultSettingsBuildingRequest.java
+++ b/maven-settings-builder/src/main/java/org/apache/maven/settings/building/DefaultSettingsBuildingRequest.java
@@ -116,10 +116,9 @@ public class DefaultSettingsBuildingRequest
if ( systemProperties != null )
{
this.systemProperties = new Properties();
- // MNG-5670 guard against ConcurrentModificationException
- for ( String key : System.getProperties().stringPropertyNames() )
- {
- this.systemProperties.put( key, System.getProperty( key ) );
+ synchronized ( systemProperties )
+ { // avoid concurrentmodification if someone else sets/removes an unrelated system property
+ this.systemProperties.putAll( systemProperties );
}
}
else