[MRESOLVER-153] Make TrackingFileManager use named locks as well

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java

@@ -27,11 +27,18 @@
 import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 
+import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.spi.locator.Service;
+import org.eclipse.aether.spi.locator.ServiceLocator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -41,100 +48,170 @@
 @Singleton
 @Named
 public final class DefaultTrackingFileManager
-    implements TrackingFileManager
+    implements TrackingFileManager, Service
 {
     private static final Logger LOGGER = LoggerFactory.getLogger( DefaultTrackingFileManager.class );
 
+    private static final String LOCK_PREFIX = "tracking:";
+
+    private NamedLockFactory namedLockFactory;
+
+    public DefaultTrackingFileManager()
+    {
+        // ctor for ServiceLocator
+    }
+
+    @Inject
+    public DefaultTrackingFileManager( final NamedLockFactorySelector selector )
+    {
+        this.namedLockFactory = selector.getSelectedNamedLockFactory();
+    }
+
+    @Override
+    public void initService( final ServiceLocator locator )
+    {
+        NamedLockFactorySelector select = Objects.requireNonNull(
+            locator.getService( NamedLockFactorySelector.class ) );
+        this.namedLockFactory = select.getSelectedNamedLockFactory();
+    }
+
+    private String getFileKey( final File file )
+    {
+        return LOCK_PREFIX + file.getAbsolutePath();
+    }
+
     @Override
     public Properties read( File file )
     {
-        FileInputStream stream = null;
-        try
+        try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) )
         {
-            if ( !file.exists() )
+            if ( lock.lockShared( NamedLockFactorySelector.TIME, NamedLockFactorySelector.TIME_UNIT ) )
             {
-                return null;
+                try
+                {
+                    FileInputStream stream = null;
+                    try
+                    {
+                        if ( !file.exists() )
+                        {
+                            return null;
+                        }
+
+                        stream = new FileInputStream( file );
+
+                        Properties props = new Properties();
+                        props.load( stream );
+
+                        return props;
+                    }
+                    catch ( IOException e )
+                    {
+                        LOGGER.warn( "Failed to read tracking file {}", file, e );
+                    }
+                    finally
+                    {
+                        close( stream, file );
+                    }
+
+                    return null;
+                }
+                finally
+                {
+                    lock.unlock();
+                }
+            }
+            else
+            {
+                throw new IllegalStateException( "Could not acquire read lock for: " + file );
             }
-
-            stream = new FileInputStream( file );
-
-            Properties props = new Properties();
-            props.load( stream );
-
-            return props;
-        }
-        catch ( IOException e )
-        {
-            LOGGER.warn( "Failed to read tracking file {}", file, e );
         }
-        finally
+        catch ( InterruptedException e )
         {
-            close( stream, file );
+            throw new IllegalStateException( e );
         }
-
-        return null;
     }
 
     @Override
     public Properties update( File file, Map<String, String> updates )
     {
-        Properties props = new Properties();
-
-        File directory = file.getParentFile();
-        if ( !directory.mkdirs() && !directory.exists() )
-        {
-            LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
-            return props;
-        }
-
-        RandomAccessFile raf = null;
-        try
+        try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) )
         {
-            raf = new RandomAccessFile( file, "rw" );
-
-            if ( file.canRead() )
+            if ( lock.lockExclusively( NamedLockFactorySelector.TIME, NamedLockFactorySelector.TIME_UNIT ) )
             {
-                byte[] buffer = new byte[(int) raf.length()];
-
-                raf.readFully( buffer );
-
-                ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
-
-                props.load( stream );
-            }
-
-            for ( Map.Entry<String, String> update : updates.entrySet() )
-            {
-                if ( update.getValue() == null )
+                try
                 {
-                    props.remove( update.getKey() );
+                    Properties props = new Properties();
+
+                    File directory = file.getParentFile();
+                    if ( !directory.mkdirs() && !directory.exists() )
+                    {
+                        LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
+                        return props;
+                    }
+
+                    RandomAccessFile raf = null;
+                    try
+                    {
+                        raf = new RandomAccessFile( file, "rw" );
+
+                        if ( file.canRead() )
+                        {
+                            byte[] buffer = new byte[(int) raf.length()];
+
+                            raf.readFully( buffer );
+
+                            ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
+
+                            props.load( stream );
+                        }
+
+                        for ( Map.Entry<String, String> update : updates.entrySet() )
+                        {
+                            if ( update.getValue() == null )
+                            {
+                                props.remove( update.getKey() );
+                            }
+                            else
+                            {
+                                props.setProperty( update.getKey(), update.getValue() );
+                            }
+                        }
+
+                        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." );
+
+                        raf.seek( 0 );
+                        raf.write( stream.toByteArray() );
+                        raf.setLength( raf.getFilePointer() );
+                    }
+                    catch ( IOException e )
+                    {
+                        LOGGER.warn( "Failed to write tracking file {}", file, e );
+                    }
+                    finally
+                    {
+                        close( raf, file );
+                    }
+
+                    return props;
                 }
-                else
+                finally
                 {
-                    props.setProperty( update.getKey(), update.getValue() );
+                    lock.unlock();
                 }
             }
-
-            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." );
-
-            raf.seek( 0 );
-            raf.write( stream.toByteArray() );
-            raf.setLength( raf.getFilePointer() );
-        }
-        catch ( IOException e )
-        {
-            LOGGER.warn( "Failed to write tracking file {}", file, e );
+            else
+            {
+                throw new IllegalStateException( "Could not acquire write lock for: " + file );
+            }
         }
-        finally
+        catch ( InterruptedException e )
         {
-            close( raf, file );
+            throw new IllegalStateException( e );
         }
-
-        return props;
     }
 
     private void close( Closeable closeable, File file )

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultArtifactResolverTest.java

@@ -38,8 +38,7 @@
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.impl.UpdateCheckManager;
 import org.eclipse.aether.impl.VersionResolver;
-import org.eclipse.aether.internal.impl.DefaultArtifactResolver;
-import org.eclipse.aether.internal.impl.DefaultUpdateCheckManager;
+import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector;
 import org.eclipse.aether.internal.test.util.TestFileProcessor;
 import org.eclipse.aether.internal.test.util.TestFileUtils;
 import org.eclipse.aether.internal.test.util.TestLocalRepositoryManager;
@@ -274,7 +273,7 @@ public void get( Collection<? extends ArtifactDownload> artifactDownloads,
         repositoryConnectorProvider.setConnector( connector );
         resolver.setUpdateCheckManager( new DefaultUpdateCheckManager()
             .setUpdatePolicyAnalyzer( new DefaultUpdatePolicyAnalyzer() )
-            .setTrackingFileManager( new DefaultTrackingFileManager() )
+            .setTrackingFileManager( new DefaultTrackingFileManager( new NamedLockFactorySelector() ) )
         );
 
         session.setResolutionErrorPolicy( new SimpleResolutionErrorPolicy( true, false ) );

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java

@@ -23,28 +23,24 @@
 
 import java.io.File;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
-import org.eclipse.aether.internal.impl.TrackingFileManager;
+import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector;
 import org.eclipse.aether.internal.test.util.TestFileUtils;
 import org.junit.Test;
 
 /**
  */
 public class DefaultTrackingFileManagerTest
 {
+    private final TrackingFileManager tfm = new DefaultTrackingFileManager( new NamedLockFactorySelector() );
 
     @Test
     public void testRead()
         throws Exception
     {
-        TrackingFileManager tfm = new DefaultTrackingFileManager();
-
         File propFile = TestFileUtils.createTempFile( "#COMMENT\nkey1=value1\nkey2 : value2" );
         Properties props = tfm.read( propFile );
 
@@ -63,8 +59,6 @@ public void testRead()
     public void testReadNoFileLeak()
         throws Exception
     {
-        TrackingFileManager tfm = new DefaultTrackingFileManager();
-
         for ( int i = 0; i < 1000; i++ )
         {
             File propFile = TestFileUtils.createTempFile( "#COMMENT\nkey1=value1\nkey2 : value2" );
@@ -77,8 +71,6 @@ public void testReadNoFileLeak()
     public void testUpdate()
         throws Exception
     {
-        TrackingFileManager tfm = new DefaultTrackingFileManager();
-
         // NOTE: The excessive repetitions are to check the update properly truncates the file
         File propFile = TestFileUtils.createTempFile( "key1=value1\nkey2 : value2\n".getBytes( StandardCharsets.UTF_8 ), 1000 );
 
@@ -100,8 +92,6 @@ public void testUpdate()
     public void testUpdateNoFileLeak()
         throws Exception
     {
-        TrackingFileManager tfm = new DefaultTrackingFileManager();
-
         Map<String, String> updates = new HashMap<>();
         updates.put( "k", "v" );
 

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java

@@ -32,7 +32,7 @@
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.impl.UpdateCheck;
-import org.eclipse.aether.internal.impl.DefaultUpdateCheckManager;
+import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector;
 import org.eclipse.aether.internal.test.util.TestFileUtils;
 import org.eclipse.aether.internal.test.util.TestUtils;
 import org.eclipse.aether.metadata.DefaultMetadata;
@@ -82,7 +82,7 @@ public void setup()
             new RemoteRepository.Builder( "id", "default", TestFileUtils.createTempDir().toURI().toURL().toString() ).build();
         manager = new DefaultUpdateCheckManager()
             .setUpdatePolicyAnalyzer( new DefaultUpdatePolicyAnalyzer() )
-            .setTrackingFileManager( new DefaultTrackingFileManager() );
+            .setTrackingFileManager( new DefaultTrackingFileManager( new NamedLockFactorySelector() ) );
         metadata =
             new DefaultMetadata( "gid", "aid", "ver", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT,
                                  metadataFile );

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManagerTest.java

@@ -31,7 +31,7 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.artifact.DefaultArtifact;
-import org.eclipse.aether.internal.impl.EnhancedLocalRepositoryManager;
+import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector;
 import org.eclipse.aether.internal.test.util.TestFileUtils;
 import org.eclipse.aether.internal.test.util.TestUtils;
 import org.eclipse.aether.metadata.DefaultMetadata;
@@ -98,7 +98,7 @@ public void setup()
 
         basedir = TestFileUtils.createTempDir( "enhanced-repo" );
         session = TestUtils.newSession();
-        trackingFileManager = new DefaultTrackingFileManager();
+        trackingFileManager = new DefaultTrackingFileManager( new NamedLockFactorySelector() );
         manager = new EnhancedLocalRepositoryManager( basedir, session, trackingFileManager );
 
         artifactFile = new File( basedir, manager.getPathForLocalArtifact( artifact ) );