You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2020/08/17 07:26:42 UTC

[maven-resolver] 01/01: [MRESOLVER-132] Remove synchronization in TrackingFileManager

This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch MRESOLVER-132
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git

commit 392d3029bdf1468695a546ee38ca30e6039cabca
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Mon Aug 17 09:20:36 2020 +0200

    [MRESOLVER-132] Remove synchronization in TrackingFileManager
---
 .../aether/internal/impl/TrackingFileManager.java  | 202 ++++++---------------
 1 file changed, 57 insertions(+), 145 deletions(-)

diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
index 8e67e8c..e31f096 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
@@ -29,14 +29,11 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.RandomAccessFile;
-import java.nio.channels.FileChannel;
-import java.nio.channels.FileLock;
-import java.nio.channels.OverlappingFileLockException;
 import java.util.Map;
 import java.util.Properties;
 
 /**
- * Manages potentially concurrent accesses to a properties file.
+ * Manages access to a properties file.
  */
 class TrackingFileManager
 {
@@ -45,35 +42,28 @@ class TrackingFileManager
 
     public Properties read( File file )
     {
-        synchronized ( getLock( file ) )
+        FileInputStream stream = null;
+        try
         {
-            FileLock lock = null;
-            FileInputStream stream = null;
-            try
+            if ( !file.exists() )
             {
-                if ( !file.exists() )
-                {
-                    return null;
-                }
+                return null;
+            }
 
-                stream = new FileInputStream( file );
+            stream = new FileInputStream( file );
 
-                lock = lock( stream.getChannel(), Math.max( 1, file.length() ), true );
+            Properties props = new Properties();
+            props.load( stream );
 
-                Properties props = new Properties();
-                props.load( stream );
-
-                return props;
-            }
-            catch ( IOException e )
-            {
-                LOGGER.warn( "Failed to read tracking file {}", file, e );
-            }
-            finally
-            {
-                release( lock, file );
-                close( stream, file );
-            }
+            return props;
+        }
+        catch ( IOException e )
+        {
+            LOGGER.warn( "Failed to read tracking file {}", file, e );
+        }
+        finally
+        {
+            close( stream, file );
         }
 
         return null;
@@ -83,82 +73,61 @@ class TrackingFileManager
     {
         Properties props = new Properties();
 
-        synchronized ( getLock( file ) )
+        File directory = file.getParentFile();
+        if ( !directory.mkdirs() && !directory.exists() )
         {
-            File directory = file.getParentFile();
-            if ( !directory.mkdirs() && !directory.exists() )
-            {
-                LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
-                return props;
-            }
+            LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
+            return props;
+        }
 
-            RandomAccessFile raf = null;
-            FileLock lock = null;
-            try
+        RandomAccessFile raf = null;
+        try
+        {
+            raf = new RandomAccessFile( file, "rw" );
+
+            if ( file.canRead() )
             {
-                raf = new RandomAccessFile( file, "rw" );
-                lock = lock( raf.getChannel(), Math.max( 1, raf.length() ), false );
+                byte[] buffer = new byte[(int) raf.length()];
 
-                if ( file.canRead() )
-                {
-                    byte[] buffer = new byte[(int) raf.length()];
+                raf.readFully( buffer );
 
-                    raf.readFully( buffer );
+                ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
 
-                    ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
+                props.load( stream );
+            }
 
-                    props.load( stream );
+            for ( Map.Entry<String, String> update : updates.entrySet() )
+            {
+                if ( update.getValue() == null )
+                {
+                    props.remove( update.getKey() );
                 }
-
-                for ( Map.Entry<String, String> update : updates.entrySet() )
+                else
                 {
-                    if ( update.getValue() == null )
-                    {
-                        props.remove( update.getKey() );
-                    }
-                    else
-                    {
-                        props.setProperty( update.getKey(), update.getValue() );
-                    }
+                    props.setProperty( update.getKey(), update.getValue() );
                 }
+            }
 
-                ByteArrayOutputStream stream = new ByteArrayOutputStream( 1024 * 2 );
+            ByteArrayOutputStream stream = new ByteArrayOutputStream( 1024 * 2 );
 
-                LOGGER.debug( "Writing tracking file {}", file );
-                props.store( stream, "NOTE: This is a Maven Resolver internal implementation file"
-                    + ", its format can be changed without prior notice." );
+            LOGGER.debug( "Writing tracking file {}", file );
+            props.store( stream, "NOTE: This is a Maven Resolver internal implementation file"
+                + ", its format can be changed without prior notice." );
 
-                raf.seek( 0 );
-                raf.write( stream.toByteArray() );
-                raf.setLength( raf.getFilePointer() );
-            }
-            catch ( IOException e )
-            {
-                LOGGER.warn( "Failed to write tracking file {}", file, e );
-            }
-            finally
-            {
-                release( lock, file );
-                close( raf, file );
-            }
+            raf.seek( 0 );
+            raf.write( stream.toByteArray() );
+            raf.setLength( raf.getFilePointer() );
         }
-
-        return props;
-    }
-
-    private void release( FileLock lock, File file )
-    {
-        if ( lock != null )
+        catch ( IOException e )
         {
-            try
-            {
-                lock.release();
-            }
-            catch ( IOException e )
-            {
-                LOGGER.warn( "Error releasing lock for tracking file {}", file, e );
-            }
+            LOGGER.warn( "Failed to write tracking file {}", file, e );
         }
+        finally
+        {
+            close( raf, file );
+        }
+
+        return props;
     }
 
     private void close( Closeable closeable, File file )
@@ -176,61 +145,4 @@ class TrackingFileManager
         }
     }
 
-    private Object getLock( File file )
-    {
-        /*
-         * NOTE: Locks held by one JVM must not overlap and using the canonical path is our best bet, still another
-         * piece of code might have locked the same file (unlikely though) or the canonical path fails to capture file
-         * identity sufficiently as is the case with Java 1.6 and symlinks on Windows.
-         */
-        try
-        {
-            return file.getCanonicalPath().intern();
-        }
-        catch ( IOException e )
-        {
-            LOGGER.warn( "Failed to canonicalize path {}", file, e );
-            // TODO This is code smell and deprecated
-            return file.getAbsolutePath().intern();
-        }
-    }
-
-    @SuppressWarnings( { "checkstyle:magicnumber" } )
-    private FileLock lock( FileChannel channel, long size, boolean shared )
-        throws IOException
-    {
-        FileLock lock = null;
-
-        for ( int attempts = 8; attempts >= 0; attempts-- )
-        {
-            try
-            {
-                lock = channel.lock( 0, size, shared );
-                break;
-            }
-            catch ( OverlappingFileLockException e )
-            {
-                if ( attempts <= 0 )
-                {
-                    throw new IOException( e );
-                }
-                try
-                {
-                    Thread.sleep( 50L );
-                }
-                catch ( InterruptedException e1 )
-                {
-                    Thread.currentThread().interrupt();
-                }
-            }
-        }
-
-        if ( lock == null )
-        {
-            throw new IOException( "Could not lock file" );
-        }
-
-        return lock;
-    }
-
 }