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

Reply via email to