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/18 09:21:34 UTC
[maven-resolver] branch MRESOLVER-132 updated (d18d783 -> 7665415)
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a change to branch MRESOLVER-132
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git.
discard d18d783 [MRESOLVER-132] Remove synchronization in TrackingFileManager
new 7665415 [MRESOLVER-132] Remove synchronization in TrackingFileManager
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (d18d783)
\
N -- N -- N refs/heads/MRESOLVER-132 (7665415)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
[maven-resolver] 01/01: [MRESOLVER-132] Remove synchronization in
TrackingFileManager
Posted by mi...@apache.org.
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 7665415dcecddd1b8f739cbc356a4cea2bbd50c7
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 );
- }
-
}