Am 2016-10-16 um 18:39 schrieb Christian Schulte:
Btw: Do you know why and where system properties are being removed?
This is still the question and I would like to have an answer for.
Am 10/16/16 um 01:40 schrieb gb...@apache.org:
Repository: maven
Updated Branches:
refs/heads/master f7c1359cf -> ace448158
[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/ace44815
Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/ace44815
Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/ace44815
Branch: refs/heads/master
Commit: ace448158131285e5ef8fb54b96dfb3d8d05f37e
Parents: f7c1359
Author: Guillaume Boué <gb...@apache.org>
Authored: Sun Oct 16 01:40:46 2016 +0200
Committer: Guillaume Boué <gb...@apache.org>
Committed: Sun Oct 16 01:40:46 2016 +0200
----------------------------------------------------------------------
.../internal/MavenRepositorySystemUtils.java | 21 +++---------
.../execution/DefaultMavenExecutionRequest.java | 4 +--
.../project/DefaultProjectBuildingRequest.java | 7 ++--
.../properties/internal/SystemProperties.java | 35 ++++++++++++--------
.../building/DefaultModelBuildingRequest.java | 5 ++-
.../DefaultSettingsBuildingRequest.java | 12 ++-----
6 files changed, 38 insertions(+), 46 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/maven/blob/ace44815/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 877c277..645fd1c 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
@@ -19,9 +19,6 @@ package org.apache.maven.repository.internal;
* under the License.
*/
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
import java.util.Properties;
import org.eclipse.aether.DefaultRepositorySystemSession;
@@ -130,22 +127,14 @@ public final class MavenRepositorySystemUtils
session.setArtifactDescriptorPolicy( new
SimpleArtifactDescriptorPolicy( true, true ) );
+ final Properties systemProperties = new Properties();
+
// MNG-5670 guard against ConcurrentModificationException
// MNG-6053 guard against key without value
- final Properties systemProperties = new Properties();
- // This relies on the fact that load/store are synchronized internally.
- try ( final ByteArrayOutputStream out = new ByteArrayOutputStream() )
- {
- System.getProperties().store( out, null );
-
- try ( final ByteArrayInputStream in = new ByteArrayInputStream(
out.toByteArray() ) )
- {
- systemProperties.load( in );
- }
- }
- catch ( final IOException e )
+ Properties sysProp = System.getProperties();
+ synchronized ( sysProp )
{
- throw new AssertionError( "Unexpected IO error copying system
properties.", e );
+ systemProperties.putAll( sysProp );
}
session.setSystemProperties( systemProperties );
http://git-wip-us.apache.org/repos/asf/maven/blob/ace44815/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/ace44815/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 97eb276..dafbefd 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
@@ -166,11 +167,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/ace44815/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 0a77376..f5630c1 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,24 +33,33 @@ public class SystemProperties
*/
public static void addSystemProperties( Properties props )
{
- for ( String key : System.getProperties().stringPropertyNames() )
- {
- String value = System.getProperty( key );
- // could be null if another thread concurrently removed this key
(MNG-6105)
- if ( value != null )
- {
- props.put( key, value );
- }
- }
+ 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/ace44815/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/ace44815/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 d917a9c..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,15 +116,9 @@ public class DefaultSettingsBuildingRequest
if ( systemProperties != null )
{
this.systemProperties = new Properties();
- // MNG-5670 guard against ConcurrentModificationException
- for ( String key : System.getProperties().stringPropertyNames() )
- {
- String value = System.getProperty( key );
- // could be null if another thread concurrently removed this
key (MNG-6105)
- if ( value != null )
- {
- this.systemProperties.put( key, value );
- }
+ synchronized ( systemProperties )
+ { // avoid concurrentmodification if someone else sets/removes an
unrelated system property
+ this.systemProperties.putAll( systemProperties );
}
}
else
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org