Skip to content

Commit 7665415

Browse files
michael-omichael-o
committed
[MRESOLVER-132] Remove synchronization in TrackingFileManager
This closes #67
1 parent 196d6a2 commit 7665415

File tree

2 files changed

+59
-200
lines changed

2 files changed

+59
-200
lines changed

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

Lines changed: 57 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,11 @@
2929
import java.io.FileInputStream;
3030
import java.io.IOException;
3131
import java.io.RandomAccessFile;
32-
import java.nio.channels.FileChannel;
33-
import java.nio.channels.FileLock;
34-
import java.nio.channels.OverlappingFileLockException;
3532
import java.util.Map;
3633
import java.util.Properties;
3734

3835
/**
39-
* Manages potentially concurrent accesses to a properties file.
36+
* Manages access to a properties file.
4037
*/
4138
class TrackingFileManager
4239
{
@@ -45,35 +42,28 @@ class TrackingFileManager
4542

4643
public Properties read( File file )
4744
{
48-
synchronized ( getLock( file ) )
45+
FileInputStream stream = null;
46+
try
4947
{
50-
FileLock lock = null;
51-
FileInputStream stream = null;
52-
try
48+
if ( !file.exists() )
5349
{
54-
if ( !file.exists() )
55-
{
56-
return null;
57-
}
50+
return null;
51+
}
5852

59-
stream = new FileInputStream( file );
53+
stream = new FileInputStream( file );
6054

61-
lock = lock( stream.getChannel(), Math.max( 1, file.length() ), true );
55+
Properties props = new Properties();
56+
props.load( stream );
6257

63-
Properties props = new Properties();
64-
props.load( stream );
65-
66-
return props;
67-
}
68-
catch ( IOException e )
69-
{
70-
LOGGER.warn( "Failed to read tracking file {}", file, e );
71-
}
72-
finally
73-
{
74-
release( lock, file );
75-
close( stream, file );
76-
}
58+
return props;
59+
}
60+
catch ( IOException e )
61+
{
62+
LOGGER.warn( "Failed to read tracking file {}", file, e );
63+
}
64+
finally
65+
{
66+
close( stream, file );
7767
}
7868

7969
return null;
@@ -83,82 +73,61 @@ public Properties update( File file, Map<String, String> updates )
8373
{
8474
Properties props = new Properties();
8575

86-
synchronized ( getLock( file ) )
76+
File directory = file.getParentFile();
77+
if ( !directory.mkdirs() && !directory.exists() )
8778
{
88-
File directory = file.getParentFile();
89-
if ( !directory.mkdirs() && !directory.exists() )
90-
{
91-
LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
92-
return props;
93-
}
79+
LOGGER.warn( "Failed to create parent directories for tracking file {}", file );
80+
return props;
81+
}
9482

95-
RandomAccessFile raf = null;
96-
FileLock lock = null;
97-
try
83+
RandomAccessFile raf = null;
84+
try
85+
{
86+
raf = new RandomAccessFile( file, "rw" );
87+
88+
if ( file.canRead() )
9889
{
99-
raf = new RandomAccessFile( file, "rw" );
100-
lock = lock( raf.getChannel(), Math.max( 1, raf.length() ), false );
90+
byte[] buffer = new byte[(int) raf.length()];
10191

102-
if ( file.canRead() )
103-
{
104-
byte[] buffer = new byte[(int) raf.length()];
92+
raf.readFully( buffer );
10593

106-
raf.readFully( buffer );
94+
ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
10795

108-
ByteArrayInputStream stream = new ByteArrayInputStream( buffer );
96+
props.load( stream );
97+
}
10998

110-
props.load( stream );
99+
for ( Map.Entry<String, String> update : updates.entrySet() )
100+
{
101+
if ( update.getValue() == null )
102+
{
103+
props.remove( update.getKey() );
111104
}
112-
113-
for ( Map.Entry<String, String> update : updates.entrySet() )
105+
else
114106
{
115-
if ( update.getValue() == null )
116-
{
117-
props.remove( update.getKey() );
118-
}
119-
else
120-
{
121-
props.setProperty( update.getKey(), update.getValue() );
122-
}
107+
props.setProperty( update.getKey(), update.getValue() );
123108
}
109+
}
124110

125-
ByteArrayOutputStream stream = new ByteArrayOutputStream( 1024 * 2 );
111+
ByteArrayOutputStream stream = new ByteArrayOutputStream( 1024 * 2 );
126112

127-
LOGGER.debug( "Writing tracking file {}", file );
128-
props.store( stream, "NOTE: This is a Maven Resolver internal implementation file"
129-
+ ", its format can be changed without prior notice." );
113+
LOGGER.debug( "Writing tracking file {}", file );
114+
props.store( stream, "NOTE: This is a Maven Resolver internal implementation file"
115+
+ ", its format can be changed without prior notice." );
130116

131-
raf.seek( 0 );
132-
raf.write( stream.toByteArray() );
133-
raf.setLength( raf.getFilePointer() );
134-
}
135-
catch ( IOException e )
136-
{
137-
LOGGER.warn( "Failed to write tracking file {}", file, e );
138-
}
139-
finally
140-
{
141-
release( lock, file );
142-
close( raf, file );
143-
}
117+
raf.seek( 0 );
118+
raf.write( stream.toByteArray() );
119+
raf.setLength( raf.getFilePointer() );
144120
}
145-
146-
return props;
147-
}
148-
149-
private void release( FileLock lock, File file )
150-
{
151-
if ( lock != null )
121+
catch ( IOException e )
152122
{
153-
try
154-
{
155-
lock.release();
156-
}
157-
catch ( IOException e )
158-
{
159-
LOGGER.warn( "Error releasing lock for tracking file {}", file, e );
160-
}
123+
LOGGER.warn( "Failed to write tracking file {}", file, e );
161124
}
125+
finally
126+
{
127+
close( raf, file );
128+
}
129+
130+
return props;
162131
}
163132

164133
private void close( Closeable closeable, File file )
@@ -176,61 +145,4 @@ private void close( Closeable closeable, File file )
176145
}
177146
}
178147

179-
private Object getLock( File file )
180-
{
181-
/*
182-
* NOTE: Locks held by one JVM must not overlap and using the canonical path is our best bet, still another
183-
* piece of code might have locked the same file (unlikely though) or the canonical path fails to capture file
184-
* identity sufficiently as is the case with Java 1.6 and symlinks on Windows.
185-
*/
186-
try
187-
{
188-
return file.getCanonicalPath().intern();
189-
}
190-
catch ( IOException e )
191-
{
192-
LOGGER.warn( "Failed to canonicalize path {}", file, e );
193-
// TODO This is code smell and deprecated
194-
return file.getAbsolutePath().intern();
195-
}
196-
}
197-
198-
@SuppressWarnings( { "checkstyle:magicnumber" } )
199-
private FileLock lock( FileChannel channel, long size, boolean shared )
200-
throws IOException
201-
{
202-
FileLock lock = null;
203-
204-
for ( int attempts = 8; attempts >= 0; attempts-- )
205-
{
206-
try
207-
{
208-
lock = channel.lock( 0, size, shared );
209-
break;
210-
}
211-
catch ( OverlappingFileLockException e )
212-
{
213-
if ( attempts <= 0 )
214-
{
215-
throw new IOException( e );
216-
}
217-
try
218-
{
219-
Thread.sleep( 50L );
220-
}
221-
catch ( InterruptedException e1 )
222-
{
223-
Thread.currentThread().interrupt();
224-
}
225-
}
226-
}
227-
228-
if ( lock == null )
229-
{
230-
throw new IOException( "Could not lock file" );
231-
}
232-
233-
return lock;
234-
}
235-
236148
}

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

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
* to you under the Apache License, Version 2.0 (the
99
* "License"); you may not use this file except in compliance
1010
* with the License. You may obtain a copy of the License at
11-
*
11+
*
1212
* http://www.apache.org/licenses/LICENSE-2.0
13-
*
13+
*
1414
* Unless required by applicable law or agreed to in writing,
1515
* software distributed under the License is distributed on an
1616
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -113,57 +113,4 @@ public void testUpdateNoFileLeak()
113113
}
114114
}
115115

116-
@Test
117-
public void testLockingOnCanonicalPath()
118-
throws Exception
119-
{
120-
final TrackingFileManager tfm = new TrackingFileManager();
121-
122-
final File propFile = TestFileUtils.createTempFile( "#COMMENT\nkey1=value1\nkey2 : value2" );
123-
124-
final List<Throwable> errors = Collections.synchronizedList( new ArrayList<Throwable>() );
125-
126-
Thread[] threads = new Thread[4];
127-
for ( int i = 0; i < threads.length; i++ )
128-
{
129-
String path = propFile.getParent();
130-
for ( int j = 0; j < i; j++ )
131-
{
132-
path += "/.";
133-
}
134-
path += "/" + propFile.getName();
135-
final File file = new File( path );
136-
137-
threads[i] = new Thread()
138-
{
139-
public void run()
140-
{
141-
try
142-
{
143-
for ( int i = 0; i < 1000; i++ )
144-
{
145-
assertNotNull( tfm.read( file ) );
146-
}
147-
}
148-
catch ( Throwable e )
149-
{
150-
errors.add( e );
151-
}
152-
}
153-
};
154-
}
155-
156-
for ( Thread thread1 : threads )
157-
{
158-
thread1.start();
159-
}
160-
161-
for ( Thread thread : threads )
162-
{
163-
thread.join();
164-
}
165-
166-
assertEquals( Collections.emptyList(), errors );
167-
}
168-
169116
}

0 commit comments

Comments
 (0)