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/29 18:58:51 UTC
[maven-resolver] branch master updated: [MRESOLVER-132] Remove
synchronization in TrackingFileManager
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new fcb6be5 [MRESOLVER-132] Remove synchronization in TrackingFileManager
fcb6be5 is described below
commit fcb6be59c5a5855573886b09c70dab80074a1d27
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Mon Aug 17 09:20:36 2020 +0200
[MRESOLVER-132] Remove synchronization in TrackingFileManager
This closes #67
---
.../aether/internal/impl/TrackingFileManager.java | 202 ++++++---------------
.../internal/impl/TrackingFileManagerTest.java | 57 +-----
2 files changed, 59 insertions(+), 200 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;
- }
-
}
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTest.java
index 6dbd21b..1593fa4 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/TrackingFileManagerTest.java
@@ -8,9 +8,9 @@ package org.eclipse.aether.internal.impl;
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -113,57 +113,4 @@ public class TrackingFileManagerTest
}
}
- @Test
- public void testLockingOnCanonicalPath()
- throws Exception
- {
- final TrackingFileManager tfm = new TrackingFileManager();
-
- final File propFile = TestFileUtils.createTempFile( "#COMMENT\nkey1=value1\nkey2 : value2" );
-
- final List<Throwable> errors = Collections.synchronizedList( new ArrayList<Throwable>() );
-
- Thread[] threads = new Thread[4];
- for ( int i = 0; i < threads.length; i++ )
- {
- String path = propFile.getParent();
- for ( int j = 0; j < i; j++ )
- {
- path += "/.";
- }
- path += "/" + propFile.getName();
- final File file = new File( path );
-
- threads[i] = new Thread()
- {
- public void run()
- {
- try
- {
- for ( int i = 0; i < 1000; i++ )
- {
- assertNotNull( tfm.read( file ) );
- }
- }
- catch ( Throwable e )
- {
- errors.add( e );
- }
- }
- };
- }
-
- for ( Thread thread1 : threads )
- {
- thread1.start();
- }
-
- for ( Thread thread : threads )
- {
- thread.join();
- }
-
- assertEquals( Collections.emptyList(), errors );
- }
-
}