diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java index fa7259009..b706bfa4a 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java @@ -55,6 +55,7 @@ import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory; import org.eclipse.aether.internal.impl.slf4j.Slf4jLoggerFactory; import org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory; +import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector; import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider; import org.eclipse.aether.spi.connector.layout.RepositoryLayoutFactory; import org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider; @@ -221,6 +222,7 @@ public DefaultServiceLocator() addService( LocalRepositoryManagerFactory.class, EnhancedLocalRepositoryManagerFactory.class ); addService( LoggerFactory.class, Slf4jLoggerFactory.class ); addService( TrackingFileManager.class, DefaultTrackingFileManager.class ); + addService( NamedLockFactorySelector.class, NamedLockFactorySelector.class ); } private Entry getEntry( Class type, boolean create ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java index 0ac8245b4..1fe9db268 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java @@ -43,10 +43,7 @@ import org.eclipse.aether.internal.impl.DefaultTrackingFileManager; import org.eclipse.aether.internal.impl.TrackingFileManager; import org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.GlobalSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.NamedSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.NoLockSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.SyncContextFactoryDelegate; +import org.eclipse.aether.internal.impl.synccontext.NamedLockFactorySelector; import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.NameMapper; @@ -77,6 +74,7 @@ import org.eclipse.aether.internal.impl.Maven2RepositoryLayoutFactory; import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory; import org.eclipse.aether.internal.impl.slf4j.Slf4jLoggerFactory; +import org.eclipse.aether.named.providers.NoopNamedLockFactory; import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider; import org.eclipse.aether.spi.connector.layout.RepositoryLayoutFactory; import org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider; @@ -160,16 +158,11 @@ protected void configure() .to( EnhancedLocalRepositoryManagerFactory.class ).in( Singleton.class ); bind( TrackingFileManager.class ).to( DefaultTrackingFileManager.class ).in( Singleton.class ); + bind( NamedLockFactorySelector.class ).in( Singleton.class ); bind( SyncContextFactory.class ).to( DefaultSyncContextFactory.class ).in( Singleton.class ); bind( org.eclipse.aether.impl.SyncContextFactory.class ) .to( org.eclipse.aether.internal.impl.synccontext.legacy.DefaultSyncContextFactory.class ) .in( Singleton.class ); - bind( SyncContextFactoryDelegate.class ).annotatedWith( Names.named( NoLockSyncContextFactory.NAME ) ) - .to( NoLockSyncContextFactory.class ).in( Singleton.class ); - bind( SyncContextFactoryDelegate.class ).annotatedWith( Names.named( GlobalSyncContextFactory.NAME ) ) - .to( GlobalSyncContextFactory.class ).in( Singleton.class ); - bind( SyncContextFactoryDelegate.class ).annotatedWith( Names.named( NamedSyncContextFactory.NAME ) ) - .to( NamedSyncContextFactory.class ).in( Singleton.class ); bind( NameMapper.class ).annotatedWith( Names.named( StaticNameMapper.NAME ) ) .to( StaticNameMapper.class ).in( Singleton.class ); @@ -178,29 +171,17 @@ protected void configure() bind( NameMapper.class ).annotatedWith( Names.named( DiscriminatingNameMapper.NAME ) ) .to( DiscriminatingNameMapper.class ).in( Singleton.class ); + bind( NamedLockFactory.class ).annotatedWith( Names.named( NoopNamedLockFactory.NAME ) ) + .to( NoopNamedLockFactory.class ).in( Singleton.class ); bind( NamedLockFactory.class ).annotatedWith( Names.named( LocalReadWriteLockNamedLockFactory.NAME ) ) - .to( LocalReadWriteLockNamedLockFactory.class ).in( Singleton.class ); + .to( LocalReadWriteLockNamedLockFactory.class ).in( Singleton.class ); bind( NamedLockFactory.class ).annotatedWith( Names.named( LocalSemaphoreNamedLockFactory.NAME ) ) - .to( LocalSemaphoreNamedLockFactory.class ).in( Singleton.class ); + .to( LocalSemaphoreNamedLockFactory.class ).in( Singleton.class ); install( new Slf4jModule() ); } - @Provides - @Singleton - Map provideSyncContextFactoryDelegates( - @Named( NoLockSyncContextFactory.NAME ) SyncContextFactoryDelegate nolock, - @Named( GlobalSyncContextFactory.NAME ) SyncContextFactoryDelegate global, - @Named( NamedSyncContextFactory.NAME ) SyncContextFactoryDelegate named ) - { - Map factories = new HashMap<>(); - factories.put( NoLockSyncContextFactory.NAME, nolock ); - factories.put( GlobalSyncContextFactory.NAME, global ); - factories.put( NamedSyncContextFactory.NAME, named ); - return Collections.unmodifiableMap( factories ); - } - @Provides @Singleton Map provideNameMappers( diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java index e6e2e653a..c4d8a7256 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java +++ b/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 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 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 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 ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java index 894d9cad4..1775c753b 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java @@ -19,68 +19,71 @@ * under the License. */ -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.SyncContext; -import org.eclipse.aether.spi.synccontext.SyncContextFactory; +import java.util.Objects; +import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; + +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactoryAdapter; +import org.eclipse.aether.spi.locator.Service; +import org.eclipse.aether.spi.locator.ServiceLocator; +import org.eclipse.aether.spi.synccontext.SyncContextFactory; /** - * Default {@link SyncContextFactory} implementation that delegates to some {@link SyncContextFactoryDelegate} - * implementation. + * Default {@link SyncContextFactory} implementation that uses named locks. */ @Singleton @Named public final class DefaultSyncContextFactory - implements SyncContextFactory + implements SyncContextFactory, Service { - private static final String SYNC_CONTEXT_FACTORY_NAME = System.getProperty( - "aether.syncContext.impl", NamedSyncContextFactory.NAME - ); - - private final SyncContextFactoryDelegate delegate; + private NamedLockFactoryAdapter namedLockFactoryAdapter; /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public DefaultSyncContextFactory( final Map delegates ) + public DefaultSyncContextFactory( final NamedLockFactorySelector selector ) { - Objects.requireNonNull( delegates ); - this.delegate = selectDelegate( delegates ); + this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( + selector.getSelectedNameMapper(), + selector.getSelectedNamedLockFactory(), + NamedLockFactorySelector.TIME, + NamedLockFactorySelector.TIME_UNIT + ); } - /** - * Default constructor - */ public DefaultSyncContextFactory() { - Map delegates = new HashMap<>( 3 ); - delegates.put( NoLockSyncContextFactory.NAME, new NoLockSyncContextFactory() ); - delegates.put( GlobalSyncContextFactory.NAME, new GlobalSyncContextFactory() ); - delegates.put( NamedSyncContextFactory.NAME, new NamedSyncContextFactory() ); - this.delegate = selectDelegate( delegates ); + // ctor for ServiceLoader } - private SyncContextFactoryDelegate selectDelegate( final Map delegates ) + @Override + public void initService( final ServiceLocator locator ) { - SyncContextFactoryDelegate delegate = delegates.get( SYNC_CONTEXT_FACTORY_NAME ); - if ( delegate == null ) - { - throw new IllegalArgumentException( "Unknown SyncContextFactory impl: " + SYNC_CONTEXT_FACTORY_NAME - + ", known ones: " + delegates.keySet() ); - } - return delegate; + NamedLockFactorySelector selector = Objects.requireNonNull( + locator.getService( NamedLockFactorySelector.class ) ); + this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( + selector.getSelectedNameMapper(), + selector.getSelectedNamedLockFactory(), + NamedLockFactorySelector.TIME, + NamedLockFactorySelector.TIME_UNIT + ); } @Override public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) { - return delegate.newInstance( session, shared ); + return namedLockFactoryAdapter.newInstance( session, shared ); + } + + @PreDestroy + public void shutdown() + { + namedLockFactoryAdapter.shutdown(); } } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/GlobalSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/GlobalSyncContextFactory.java deleted file mode 100644 index 047132eae..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/GlobalSyncContextFactory.java +++ /dev/null @@ -1,94 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * 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 - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import java.util.Collection; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - -import javax.inject.Named; -import javax.inject.Singleton; - -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.SyncContext; -import org.eclipse.aether.artifact.Artifact; -import org.eclipse.aether.metadata.Metadata; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * A singleton factory to create synchronization contexts using a global lock based on - * {@link ReentrantReadWriteLock}. Explicit artifacts and metadata passed are ignored. - *

- * Note: This component is still considered to be experimental, use with caution! - */ -@Singleton -@Named( GlobalSyncContextFactory.NAME ) -public class GlobalSyncContextFactory - implements SyncContextFactoryDelegate -{ - public static final String NAME = "global"; - - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - - @Override - public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) - { - return new GlobalSyncContext( shared ? lock.readLock() : lock.writeLock(), shared ); - } - - private static class GlobalSyncContext - implements SyncContext - { - private static final Logger LOGGER = LoggerFactory.getLogger( GlobalSyncContext.class ); - - private final Lock lock; - private final boolean shared; - private int lockHoldCount; - - private GlobalSyncContext( final Lock lock, final boolean shared ) - { - this.lock = lock; - this.shared = shared; - } - - @Override - public void acquire( final Collection artifact, - final Collection metadata ) - { - LOGGER.trace( "Acquiring global {} lock (currently held: {})", - shared ? "read" : "write", lockHoldCount ); - lock.lock(); - lockHoldCount++; - } - - @Override - public void close() - { - while ( lockHoldCount > 0 ) - { - LOGGER.trace( "Releasing global {} lock (currently held: {})", - shared ? "read" : "write", lockHoldCount ); - lock.unlock(); - lockHoldCount--; - } - } - } -} \ No newline at end of file diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java similarity index 58% rename from maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java rename to maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java index ba47c8d45..190950fc8 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java @@ -19,106 +19,115 @@ * under the License. */ -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.SyncContext; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; + import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.NameMapper; -import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactoryAdapter; import org.eclipse.aether.internal.impl.synccontext.named.StaticNameMapper; import org.eclipse.aether.named.NamedLockFactory; import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; - -import javax.annotation.PreDestroy; -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.TimeUnit; +import org.eclipse.aether.named.providers.NoopNamedLockFactory; /** - * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} - * implementation at creation. + * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially + * all the named locks configuration is here. */ @Singleton -@Named( NamedSyncContextFactory.NAME ) -public final class NamedSyncContextFactory - implements SyncContextFactoryDelegate +@Named +public final class NamedLockFactorySelector { - public static final String NAME = "named"; + public static final long TIME = Long.getLong( + "aether.syncContext.named.time", 30L + ); + + public static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty( + "aether.syncContext.named.time.unit", TimeUnit.SECONDS.name() + ) ); private static final String FACTORY_NAME = System.getProperty( - "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME ); - private static final String NAME_MAPPER = System.getProperty( + private static final String NAME_MAPPER_NAME = System.getProperty( "aether.syncContext.named.nameMapper", GAVNameMapper.NAME ); - private static final long TIME = Long.getLong( - "aether.syncContext.named.time", 30L - ); - - private static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty( - "aether.syncContext.named.time.unit", TimeUnit.SECONDS.name() - ) ); + private final NamedLockFactory namedLockFactory; - private final NamedLockFactoryAdapter namedLockFactoryAdapter; + private final NameMapper nameMapper; /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public NamedSyncContextFactory( final Map nameMappers, - final Map factories ) + public NamedLockFactorySelector( final Map factories, + final Map nameMappers ) { - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, factories ); + this.namedLockFactory = selectNamedLockFactory( factories ); + this.nameMapper = selectNameMapper( nameMappers ); } /** - * Default constructor + * Default constructor for ServiceLocator. */ - public NamedSyncContextFactory() + public NamedLockFactorySelector() { + Map factories = new HashMap<>(); + factories.put( NoopNamedLockFactory.NAME, new NoopNamedLockFactory() ); + factories.put( LocalReadWriteLockNamedLockFactory.NAME, new LocalReadWriteLockNamedLockFactory() ); + factories.put( LocalSemaphoreNamedLockFactory.NAME, new LocalSemaphoreNamedLockFactory() ); + this.namedLockFactory = selectNamedLockFactory( factories ); + Map nameMappers = new HashMap<>(); nameMappers.put( StaticNameMapper.NAME, new StaticNameMapper() ); nameMappers.put( GAVNameMapper.NAME, new GAVNameMapper() ); nameMappers.put( DiscriminatingNameMapper.NAME, new DiscriminatingNameMapper( new GAVNameMapper() ) ); - Map factories = new HashMap<>(); - factories.put( LocalReadWriteLockNamedLockFactory.NAME, new LocalReadWriteLockNamedLockFactory() ); - factories.put( LocalSemaphoreNamedLockFactory.NAME, new LocalSemaphoreNamedLockFactory() ); - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, factories ); + this.nameMapper = selectNameMapper( nameMappers ); } - private static NamedLockFactoryAdapter selectAndAdapt( final Map nameMappers, - final Map factories ) + /** + * Returns the selected {@link NamedLockFactory}, never null. + */ + public NamedLockFactory getSelectedNamedLockFactory() + { + return namedLockFactory; + } + + /** + * Returns the selected {@link NameMapper}, never null. + */ + public NameMapper getSelectedNameMapper() + { + return nameMapper; + } + + private static NamedLockFactory selectNamedLockFactory( final Map factories ) { - NameMapper nameMapper = nameMappers.get( NAME_MAPPER ); - if ( nameMapper == null ) - { - throw new IllegalArgumentException( "Unknown NameMapper name: " + NAME_MAPPER - + ", known ones: " + nameMappers.keySet() ); - } NamedLockFactory factory = factories.get( FACTORY_NAME ); if ( factory == null ) { throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + FACTORY_NAME - + ", known ones: " + factories.keySet() ); + + ", known ones: " + factories.keySet() ); } - return new NamedLockFactoryAdapter( nameMapper, factory, TIME, TIME_UNIT ); - } - - @Override - public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) - { - return namedLockFactoryAdapter.newInstance( session, shared ); + return factory; } - @PreDestroy - public void shutdown() + private static NameMapper selectNameMapper( final Map nameMappers ) { - namedLockFactoryAdapter.shutdown(); + NameMapper nameMapper = nameMappers.get( NAME_MAPPER_NAME ); + if ( nameMapper == null ) + { + throw new IllegalArgumentException( "Unknown NameMapper name: " + NAME_MAPPER_NAME + + ", known ones: " + nameMappers.keySet() ); + } + return nameMapper; } } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java deleted file mode 100644 index 10564d7e7..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java +++ /dev/null @@ -1,63 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * 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 - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import java.util.Collection; - -import javax.inject.Named; -import javax.inject.Singleton; - -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.SyncContext; -import org.eclipse.aether.artifact.Artifact; -import org.eclipse.aether.metadata.Metadata; - -/** - * A factory to create synchronization contexts. This default implementation does not provide any real - * synchronization but merely completes the repository system. - */ -@Singleton -@Named( NoLockSyncContextFactory.NAME ) -public class NoLockSyncContextFactory - implements SyncContextFactoryDelegate -{ - public static final String NAME = "nolock"; - - @Override - public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) - { - return new DefaultSyncContext(); - } - - private static class DefaultSyncContext - implements SyncContext - { - @Override - public void acquire( final Collection artifact, - final Collection metadata ) - { - } - - @Override - public void close() - { - } - } -} \ No newline at end of file diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryDelegate.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryDelegate.java deleted file mode 100644 index 9f768f329..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryDelegate.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * 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 - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import org.eclipse.aether.spi.synccontext.SyncContextFactory; - -/** - * Internal marker interface to mark internal implementations for {@link SyncContextFactory}. - */ -public interface SyncContextFactoryDelegate extends SyncContextFactory -{ -} diff --git a/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm b/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm index 54697a5a5..d1982912a 100644 --- a/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm +++ b/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm @@ -19,7 +19,7 @@ specific language governing permissions and limitations under the License. --> -In this module there are several implementations of `SyncContextFactory`. The `SyncContextFactory` component is +This module among others, implements `SyncContextFactory`. The `SyncContextFactory` component is responsible to synchronize (coordinate) access to shared resources in Maven Resolver internals. Some examples: - single threaded (ST) build on a single host: given resolution by default happens in multi threaded (MT) way, files in a local repository must be protected from simultaneous access. In this case, it is enough for coordination to @@ -38,22 +38,8 @@ a cross process, and in MP multi-host case a cross process and cross host coordi The defaults in Maven Resolver cover ST and MT cases out of the box (by default). -${esc.hash}${esc.hash} Provided Implementations - -This module provides a default implementation for the interface `org.eclipse.aether.spi.synccontext.SyncContextFactory` -that is able to delegate to three implementations: - -- `named` (the default) is backed by named locks, and provide locks for MT and MP builds. -- `global` (was experimental in pre 1.7.0 version) is using a single JVM - `java.util.concurrent.locks.ReentrantReadWriteLock` to protect read and write actions, hence suitable in MT - builds only. But, even in MT builds it is inferior from `named` due congestion on single lock instance. -- `nolock` (was default in pre 1.7.0 version) is actually a no-op implementation. It is fundamentally flawed, as - even single threaded builds may produce concurrency issues (see MRESOLVER-153). Should not be used by end-users, - is left for "R&D" reasons only. - -To choose which `SyncContextFactory` you want to use, the `aether.syncContext.impl` JVM system property should be set -to some value from those above. In case of wrong value, the factory constructor, hence, Maven itself will fail to -initialize. The default value used if none is set by user is `named`. +This module implementation for the interface `org.eclipse.aether.spi.synccontext.SyncContextFactory` +uses resolver named locks. ${esc.hash}${esc.hash} Configuration Options for "named" `SyncContextFactory` @@ -65,17 +51,19 @@ You can control and configure several aspects of `NamedSyncContextFactory`: - `aether.syncContext.named.time.unit` (optional, default is `java.util.concurrent.TimeUnit.SECONDS`): the time unit of time value. -For the `aether.syncContext.named.nameMapper` property following values are allowed: - -- `discriminating` (default), uses hostname + local repo + GAV to create unique lock names for artifacts. -- `gav` uses GAV to create unique lock names for artifacts and metadata. Is not unique if multiple local repositories are involved. -- `static` uses static (same) string as lock name for any input, effectively providing functionality same as "global" - locking SyncContextFactory. - For the `aether.syncContext.named.factory` property following values are allowed: - `rwlock-local` (default), uses JVM `ReentrantReadWriteLock` per lock name, usable for MT builds. - `semaphore-local`, uses JVM `Semaphore` per lock name, usable for MT builds. +- `noop`, implement no-op locking (no locking). For experimenting only. Has same functionality as old "nolock" + SyncContextFactory implementation. + +For the `aether.syncContext.named.nameMapper` property following values are allowed: + +- `discriminating` (default), uses hostname + local repo + GAV to create unique lock names for artifacts. +- `gav` uses GAV to create unique lock names for artifacts and metadata. Is not unique if multiple local repositories are involved. +- `static` uses static (same) string as lock name for any input. Effectively providing functionality same as old + "global" locking SyncContextFactory. Extra values for factory (these need extra setup and will work with Sisu DI only): diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultArtifactResolverTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultArtifactResolverTest.java index 3c27f9854..13f3de5cb 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultArtifactResolverTest.java +++ b/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 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 ) ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java index e3745aa68..ac241cb58 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java @@ -23,14 +23,11 @@ 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; @@ -38,13 +35,12 @@ */ 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 updates = new HashMap<>(); updates.put( "k", "v" ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java index 111aca754..e9261b609 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java +++ b/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 ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManagerTest.java index 1dae78941..598e712db 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManagerTest.java +++ b/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 ) ); diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java new file mode 100644 index 000000000..66b45ed6c --- /dev/null +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java @@ -0,0 +1,71 @@ +package org.eclipse.aether.named.providers; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * 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 + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.util.concurrent.TimeUnit; + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.eclipse.aether.named.support.NamedLockFactorySupport; +import org.eclipse.aether.named.support.NamedLockSupport; + +/** + * A no-op lock factory, that creates no-op locks. + */ +@Singleton +@Named( NoopNamedLockFactory.NAME ) +public class NoopNamedLockFactory + extends NamedLockFactorySupport +{ + public static final String NAME = "noop"; + + @Override + protected NoopNamedLock createLock( final String name ) + { + return new NoopNamedLock( name, this ); + } + + private static final class NoopNamedLock extends NamedLockSupport + { + private NoopNamedLock( final String name, final NamedLockFactorySupport factory ) + { + super( name, factory ); + } + + @Override + public boolean lockShared( final long time, final TimeUnit unit ) + { + return true; + } + + @Override + public boolean lockExclusively( final long time, final TimeUnit unit ) + { + return true; + } + + @Override + public void unlock() + { + // no-op + } + } +}