Btw: Do you know why and where system properties are being removed?

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

Reply via email to