From 968d2dbc1057d7959ef1fcfa6d4637e081e25c28 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 14:38:08 +0200 Subject: [PATCH 01/10] [MRESOLVER-153] Make TrackingFileManager use NamedLocks Step1: drop all SyncContextFactory implementations and indirections, redo the same functionality using NamedLocks. --- .../aether/impl/guice/AetherModule.java | 24 ---- .../DefaultSyncContextFactory.java | 122 +++++++++++++---- .../synccontext/GlobalSyncContextFactory.java | 94 ------------- .../synccontext/NamedSyncContextFactory.java | 124 ------------------ .../SyncContextFactoryDelegate.java | 29 ---- .../NoopNameMapper.java} | 38 ++---- 6 files changed, 107 insertions(+), 324 deletions(-) delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/GlobalSyncContextFactory.java delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryDelegate.java rename maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/{NoLockSyncContextFactory.java => named/NoopNameMapper.java} (54%) 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..c710f311f 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,6 @@ 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.named.GAVNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.NameMapper; @@ -164,12 +160,6 @@ protected void configure() 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 ); @@ -187,20 +177,6 @@ protected void configure() } - @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/synccontext/DefaultSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java index 894d9cad4..4b169cdf0 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,136 @@ * under the License. */ -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.SyncContext; -import org.eclipse.aether.spi.synccontext.SyncContextFactory; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +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.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.NoopNameMapper; +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 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, but supports "presets" for "global" and + * "nolock" behaviour as well. */ @Singleton @Named public final class DefaultSyncContextFactory implements SyncContextFactory { - private static final String SYNC_CONTEXT_FACTORY_NAME = System.getProperty( - "aether.syncContext.impl", NamedSyncContextFactory.NAME + private static final String SYNC_CONTEXT_FACTORY_NAME = System.getProperty( "aether.syncContext.impl" ); + + public static final String NAMED = "named"; + + public static final String GLOBAL = "global"; + + public static final String NOLOCK = "nolock"; + + private static final String FACTORY_NAME = System.getProperty( + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME ); - private final SyncContextFactoryDelegate delegate; + private static final String NAME_MAPPER = 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 NamedLockFactoryAdapter namedLockFactoryAdapter; /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public DefaultSyncContextFactory( final Map delegates ) + public DefaultSyncContextFactory( final Map nameMappers, + final Map factories ) { - Objects.requireNonNull( delegates ); - this.delegate = selectDelegate( delegates ); + this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, factories ); } /** - * Default constructor + * Default constructor for ServiceLocator. */ 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 ); + Map nameMappers = new HashMap<>(); + nameMappers.put( NoopNameMapper.NAME, new NoopNameMapper() ); + 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 ); } - private SyncContextFactoryDelegate selectDelegate( final Map delegates ) + private static NamedLockFactoryAdapter selectAndAdapt( final Map nameMappers, + final Map factories ) { - SyncContextFactoryDelegate delegate = delegates.get( SYNC_CONTEXT_FACTORY_NAME ); - if ( delegate == null ) + String nameMapperName = NAME_MAPPER; + String factoryName = FACTORY_NAME; + if ( SYNC_CONTEXT_FACTORY_NAME != null && !SYNC_CONTEXT_FACTORY_NAME.equals( NAMED ) ) { - throw new IllegalArgumentException( "Unknown SyncContextFactory impl: " + SYNC_CONTEXT_FACTORY_NAME - + ", known ones: " + delegates.keySet() ); + switch ( SYNC_CONTEXT_FACTORY_NAME ) + { + case GLOBAL: + nameMapperName = StaticNameMapper.NAME; + break; + case NOLOCK: + nameMapperName = NoopNameMapper.NAME; + break; + default: + throw new IllegalArgumentException( "Unknown SyncContextFactory impl: " + SYNC_CONTEXT_FACTORY_NAME + + ", known ones: " + NAMED + ", " + GLOBAL + ", " + NOLOCK ); + } + } - return delegate; + + NameMapper nameMapper = nameMappers.get( nameMapperName ); + if ( nameMapper == null ) + { + throw new IllegalArgumentException( "Unknown NameMapper name: " + nameMapperName + + ", known ones: " + nameMappers.keySet() ); + } + NamedLockFactory factory = factories.get( factoryName ); + if ( factory == null ) + { + throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + factoryName + + ", known ones: " + factories.keySet() ); + } + + return new NamedLockFactoryAdapter( nameMapper, factory, TIME, 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/NamedSyncContextFactory.java deleted file mode 100644 index ba47c8d45..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java +++ /dev/null @@ -1,124 +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.RepositorySystemSession; -import org.eclipse.aether.SyncContext; -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; - -/** - * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} - * implementation at creation. - */ -@Singleton -@Named( NamedSyncContextFactory.NAME ) -public final class NamedSyncContextFactory - implements SyncContextFactoryDelegate -{ - public static final String NAME = "named"; - - private static final String FACTORY_NAME = System.getProperty( - "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME - ); - - private static final String NAME_MAPPER = 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 NamedLockFactoryAdapter namedLockFactoryAdapter; - - /** - * Constructor used with DI, where factories are injected and selected based on key. - */ - @Inject - public NamedSyncContextFactory( final Map nameMappers, - final Map factories ) - { - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, factories ); - } - - /** - * Default constructor - */ - public NamedSyncContextFactory() - { - 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 ); - } - - private static NamedLockFactoryAdapter selectAndAdapt( final Map nameMappers, - 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() ); - } - return new NamedLockFactoryAdapter( nameMapper, factory, TIME, TIME_UNIT ); - } - - @Override - public SyncContext newInstance( final RepositorySystemSession session, final boolean 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/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/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NoopNameMapper.java similarity index 54% rename from maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NoLockSyncContextFactory.java rename to maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NoopNameMapper.java index 10564d7e7..3fbedb38a 100644 --- 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/named/NoopNameMapper.java @@ -1,4 +1,4 @@ -package org.eclipse.aether.internal.impl.synccontext; +package org.eclipse.aether.internal.impl.synccontext.named; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -20,44 +20,30 @@ */ import java.util.Collection; +import java.util.Collections; 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. + * No-op {@link NameMapper}, always returns empty collection. */ @Singleton -@Named( NoLockSyncContextFactory.NAME ) -public class NoLockSyncContextFactory - implements SyncContextFactoryDelegate +@Named( NoopNameMapper.NAME ) +public class NoopNameMapper + implements NameMapper { - public static final String NAME = "nolock"; + public static final String NAME = "noop"; @Override - public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) + public Collection nameLocks( final RepositorySystemSession session, + final Collection artifacts, + final Collection metadatas ) { - return new DefaultSyncContext(); + return Collections.emptyList(); } - - 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 +} From 0e254bbe75fdfa01de1a5dac38dbfb93491290d4 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 14:46:25 +0200 Subject: [PATCH 02/10] Pull out NamedLockFactory selection and expose selected one. --- .../DefaultSyncContextFactory.java | 28 ++----- .../synccontext/NamedLockFactorySelector.java | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java 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 4b169cdf0..38ec8496a 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 @@ -36,9 +36,6 @@ import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactoryAdapter; import org.eclipse.aether.internal.impl.synccontext.named.NoopNameMapper; 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 org.eclipse.aether.spi.synccontext.SyncContextFactory; /** @@ -58,10 +55,6 @@ public final class DefaultSyncContextFactory public static final String NOLOCK = "nolock"; - private static final String FACTORY_NAME = System.getProperty( - "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME - ); - private static final String NAME_MAPPER = System.getProperty( "aether.syncContext.named.nameMapper", GAVNameMapper.NAME ); @@ -81,9 +74,9 @@ public final class DefaultSyncContextFactory */ @Inject public DefaultSyncContextFactory( final Map nameMappers, - final Map factories ) + final NamedLockFactorySelector selector ) { - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, factories ); + this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, selector ); } /** @@ -96,17 +89,14 @@ public DefaultSyncContextFactory() 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 ); + NamedLockFactorySelector selector = new NamedLockFactorySelector(); + this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, selector ); } private static NamedLockFactoryAdapter selectAndAdapt( final Map nameMappers, - final Map factories ) + final NamedLockFactorySelector selector ) { String nameMapperName = NAME_MAPPER; - String factoryName = FACTORY_NAME; if ( SYNC_CONTEXT_FACTORY_NAME != null && !SYNC_CONTEXT_FACTORY_NAME.equals( NAMED ) ) { switch ( SYNC_CONTEXT_FACTORY_NAME ) @@ -130,14 +120,8 @@ private static NamedLockFactoryAdapter selectAndAdapt( final Map factories ) + { + this.namedLockFactory = select( factories ); + } + + /** + * Default constructor for ServiceLocator. + */ + public NamedLockFactorySelector() + { + Map factories = new HashMap<>(); + factories.put( LocalReadWriteLockNamedLockFactory.NAME, new LocalReadWriteLockNamedLockFactory() ); + factories.put( LocalSemaphoreNamedLockFactory.NAME, new LocalSemaphoreNamedLockFactory() ); + this.namedLockFactory = select( factories ); + } + + /** + * Returns the selected {@link NamedLockFactory}, never null. + */ + public NamedLockFactory getNamedLockFactory() + { + return namedLockFactory; + } + + private static NamedLockFactory select( final Map factories ) + { + NamedLockFactory factory = factories.get( FACTORY_NAME ); + if ( factory == null ) + { + throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + FACTORY_NAME + + ", known ones: " + factories.keySet() ); + } + return factory; + } +} From 153c81b6042be6175769d8d63889a6d60a3a3e49 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 14:58:33 +0200 Subject: [PATCH 03/10] Step3: tie up things --- .../aether/impl/DefaultServiceLocator.java | 2 ++ .../impl/DefaultTrackingFileManager.java | 29 ++++++++++++++++++- .../DefaultSyncContextFactory.java | 2 +- .../synccontext/NamedLockFactorySelector.java | 2 +- .../impl/DefaultArtifactResolverTest.java | 5 ++-- .../impl/DefaultTrackingFileManagerTest.java | 14 ++------- .../impl/DefaultUpdateCheckManagerTest.java | 4 +-- .../EnhancedLocalRepositoryManagerTest.java | 4 +-- 8 files changed, 40 insertions(+), 22 deletions(-) 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/internal/impl/DefaultTrackingFileManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java index e6e2e653a..56edf0fda 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,17 @@ 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.NamedLockFactory; +import org.eclipse.aether.spi.locator.Service; +import org.eclipse.aether.spi.locator.ServiceLocator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,10 +47,31 @@ @Singleton @Named public final class DefaultTrackingFileManager - implements TrackingFileManager + implements TrackingFileManager, Service { private static final Logger LOGGER = LoggerFactory.getLogger( DefaultTrackingFileManager.class ); + private NamedLockFactory namedLockFactory; + + public DefaultTrackingFileManager() + { + // ctor for ServiceLocator + } + + @Inject + public DefaultTrackingFileManager( final NamedLockFactorySelector selector ) + { + this.namedLockFactory = selector.getSelected(); + } + + @Override + public void initService( final ServiceLocator locator ) + { + NamedLockFactorySelector select = Objects.requireNonNull( + locator.getService( NamedLockFactorySelector.class ) ); + this.namedLockFactory = select.getSelected(); + } + @Override public Properties read( 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 38ec8496a..1c65e3a23 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 @@ -121,7 +121,7 @@ private static NamedLockFactoryAdapter selectAndAdapt( final Map 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 ) ); From 3f1a83113bf4520951a9a6b40e2bd25759447cdb Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 15:18:50 +0200 Subject: [PATCH 04/10] Step4: use locking --- .../impl/DefaultTrackingFileManager.java | 185 +++++++++++------- .../DefaultSyncContextFactory.java | 4 +- 2 files changed, 121 insertions(+), 68 deletions(-) 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 56edf0fda..5c00386a5 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 @@ -35,12 +35,16 @@ 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; +import static org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory.TIME; +import static org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory.TIME_UNIT; + /** * Manages access to a properties file. */ @@ -51,6 +55,8 @@ public final class DefaultTrackingFileManager { private static final Logger LOGGER = LoggerFactory.getLogger( DefaultTrackingFileManager.class ); + private static final String LOCK_PREFIX = "tracking:"; + private NamedLockFactory namedLockFactory; public DefaultTrackingFileManager() @@ -72,96 +78,143 @@ public void initService( final ServiceLocator locator ) this.namedLockFactory = select.getSelected(); } + private String getFileKey( final File file ) + { + return LOCK_PREFIX + "foo"; // hash file abs path? + } + @Override public Properties read( File file ) { - FileInputStream stream = null; - try + try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) ) { - if ( !file.exists() ) + if ( lock.lockShared( TIME, TIME_UNIT ) ) + { + 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 { - return null; + throw new IllegalStateException( "Could not acquire shared lock for: " + file ); } - - stream = new FileInputStream( file ); - - Properties props = new Properties(); - props.load( stream ); - - return props; } - catch ( IOException e ) + catch ( InterruptedException e ) { - LOGGER.warn( "Failed to read tracking file {}", file, e ); + throw new IllegalStateException( e ); } - finally - { - close( stream, file ); - } - - return null; } @Override public Properties update( File file, Map updates ) { - Properties props = new Properties(); - - File directory = file.getParentFile(); - if ( !directory.mkdirs() && !directory.exists() ) + try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) ) { - 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 ( lock.lockExclusively( TIME, TIME_UNIT ) ) { - 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 shared 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 1c65e3a23..21ca29058 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 @@ -59,11 +59,11 @@ public final class DefaultSyncContextFactory "aether.syncContext.named.nameMapper", GAVNameMapper.NAME ); - private static final long TIME = Long.getLong( + public static final long TIME = Long.getLong( "aether.syncContext.named.time", 30L ); - private static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty( + public static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty( "aether.syncContext.named.time.unit", TimeUnit.SECONDS.name() ) ); From c3ed1279795251384e739179dbf680cad988c1bf Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 16:30:37 +0200 Subject: [PATCH 05/10] Drop noop namemapeer and replace it with nooplock Sounds more correct. --- .../aether/impl/guice/AetherModule.java | 7 +- .../impl/DefaultTrackingFileManager.java | 11 +-- .../DefaultSyncContextFactory.java | 94 +++++-------------- .../synccontext/NamedLockFactorySelector.java | 61 ++++++++++-- .../synccontext/named/NoopNameMapper.java | 49 ---------- .../named/providers/NoopNamedLockFactory.java | 71 ++++++++++++++ 6 files changed, 158 insertions(+), 135 deletions(-) delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NoopNameMapper.java create mode 100644 maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java 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 c710f311f..0fa915cf4 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 @@ -73,6 +73,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; @@ -168,10 +169,12 @@ 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() ); 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 5c00386a5..87e8e8c92 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 @@ -42,9 +42,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory.TIME; -import static org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory.TIME_UNIT; - /** * Manages access to a properties file. */ @@ -67,7 +64,7 @@ public DefaultTrackingFileManager() @Inject public DefaultTrackingFileManager( final NamedLockFactorySelector selector ) { - this.namedLockFactory = selector.getSelected(); + this.namedLockFactory = selector.getSelectedNamedLockFactory(); } @Override @@ -75,7 +72,7 @@ public void initService( final ServiceLocator locator ) { NamedLockFactorySelector select = Objects.requireNonNull( locator.getService( NamedLockFactorySelector.class ) ); - this.namedLockFactory = select.getSelected(); + this.namedLockFactory = select.getSelectedNamedLockFactory(); } private String getFileKey( final File file ) @@ -88,7 +85,7 @@ public Properties read( File file ) { try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) ) { - if ( lock.lockShared( TIME, TIME_UNIT ) ) + if ( lock.lockShared( NamedLockFactorySelector.TIME, NamedLockFactorySelector.TIME_UNIT ) ) { try { @@ -139,7 +136,7 @@ public Properties update( File file, Map updates ) { try ( NamedLock lock = namedLockFactory.getLock( getFileKey( file ) ) ) { - if ( lock.lockExclusively( TIME, TIME_UNIT ) ) + if ( lock.lockExclusively( NamedLockFactorySelector.TIME, NamedLockFactorySelector.TIME_UNIT ) ) { try { 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 21ca29058..09c90ab6d 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,9 +19,7 @@ * under the License. */ -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.TimeUnit; +import java.util.Objects; import javax.annotation.PreDestroy; import javax.inject.Inject; @@ -30,12 +28,9 @@ import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.SyncContext; -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.NoopNameMapper; -import org.eclipse.aether.internal.impl.synccontext.named.StaticNameMapper; +import org.eclipse.aether.spi.locator.Service; +import org.eclipse.aether.spi.locator.ServiceLocator; import org.eclipse.aether.spi.synccontext.SyncContextFactory; /** @@ -45,83 +40,40 @@ @Singleton @Named public final class DefaultSyncContextFactory - implements SyncContextFactory + implements SyncContextFactory, Service { - private static final String SYNC_CONTEXT_FACTORY_NAME = System.getProperty( "aether.syncContext.impl" ); - - public static final String NAMED = "named"; - - public static final String GLOBAL = "global"; - - public static final String NOLOCK = "nolock"; - - private static final String NAME_MAPPER = System.getProperty( - "aether.syncContext.named.nameMapper", GAVNameMapper.NAME - ); - - 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 final NamedLockFactoryAdapter namedLockFactoryAdapter; + private NamedLockFactoryAdapter namedLockFactoryAdapter; /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public DefaultSyncContextFactory( final Map nameMappers, - final NamedLockFactorySelector selector ) + public DefaultSyncContextFactory( final NamedLockFactorySelector selector ) { - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, selector ); + this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( + selector.getSelectedNameMapper(), + selector.getSelectedNamedLockFactory(), + NamedLockFactorySelector.TIME, + NamedLockFactorySelector.TIME_UNIT + ); } - /** - * Default constructor for ServiceLocator. - */ public DefaultSyncContextFactory() { - Map nameMappers = new HashMap<>(); - nameMappers.put( NoopNameMapper.NAME, new NoopNameMapper() ); - nameMappers.put( StaticNameMapper.NAME, new StaticNameMapper() ); - nameMappers.put( GAVNameMapper.NAME, new GAVNameMapper() ); - nameMappers.put( DiscriminatingNameMapper.NAME, new DiscriminatingNameMapper( new GAVNameMapper() ) ); - NamedLockFactorySelector selector = new NamedLockFactorySelector(); - this.namedLockFactoryAdapter = selectAndAdapt( nameMappers, selector ); + // ctor for ServiceLoader } - private static NamedLockFactoryAdapter selectAndAdapt( final Map nameMappers, - final NamedLockFactorySelector selector ) + @Override + public void initService( final ServiceLocator locator ) { - String nameMapperName = NAME_MAPPER; - if ( SYNC_CONTEXT_FACTORY_NAME != null && !SYNC_CONTEXT_FACTORY_NAME.equals( NAMED ) ) - { - switch ( SYNC_CONTEXT_FACTORY_NAME ) - { - case GLOBAL: - nameMapperName = StaticNameMapper.NAME; - break; - case NOLOCK: - nameMapperName = NoopNameMapper.NAME; - break; - default: - throw new IllegalArgumentException( "Unknown SyncContextFactory impl: " + SYNC_CONTEXT_FACTORY_NAME - + ", known ones: " + NAMED + ", " + GLOBAL + ", " + NOLOCK ); - } - - } - - NameMapper nameMapper = nameMappers.get( nameMapperName ); - if ( nameMapper == null ) - { - throw new IllegalArgumentException( "Unknown NameMapper name: " + nameMapperName - + ", known ones: " + nameMappers.keySet() ); - } - - return new NamedLockFactoryAdapter( nameMapper, selector.getSelected(), TIME, TIME_UNIT ); + NamedLockFactorySelector selector = Objects.requireNonNull( + locator.getService( NamedLockFactorySelector.class ) ); + this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( + selector.getSelectedNameMapper(), + selector.getSelectedNamedLockFactory(), + NamedLockFactorySelector.TIME, + NamedLockFactorySelector.TIME_UNIT + ); } @Override diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java index 9f72b022f..63f69f7c5 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java @@ -21,35 +21,58 @@ 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.StaticNameMapper; import org.eclipse.aether.named.NamedLockFactory; import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; +import org.eclipse.aether.named.providers.NoopNamedLockFactory; /** - * Selector for {@link NamedLockFactory} that selects and exposes selected one. + * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentiall + * all the configuration is here. */ @Singleton @Named public final class NamedLockFactorySelector { + 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 ); + private static final String NAME_MAPPER_NAME = System.getProperty( + "aether.syncContext.named.nameMapper", GAVNameMapper.NAME + ); + private final NamedLockFactory namedLockFactory; + private final NameMapper nameMapper; + /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public NamedLockFactorySelector( final Map factories ) + public NamedLockFactorySelector( final Map factories, + final Map nameMappers ) { - this.namedLockFactory = select( factories ); + this.namedLockFactory = selectNamedLockFactory( factories ); + this.nameMapper = selectNameMapper( nameMappers ); } /** @@ -58,20 +81,35 @@ public NamedLockFactorySelector( final Map factories ) 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 = select( factories ); + 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() ) ); + this.nameMapper = selectNameMapper( nameMappers ); } /** * Returns the selected {@link NamedLockFactory}, never null. */ - public NamedLockFactory getSelected() + public NamedLockFactory getSelectedNamedLockFactory() { return namedLockFactory; } - private static NamedLockFactory select( final Map factories ) + /** + * Returns the selected {@link NamedLockFactory}, never null. + */ + public NameMapper getSelectedNameMapper() + { + return nameMapper; + } + + private static NamedLockFactory selectNamedLockFactory( final Map factories ) { NamedLockFactory factory = factories.get( FACTORY_NAME ); if ( factory == null ) @@ -81,4 +119,15 @@ private static NamedLockFactory select( final Map fact } return factory; } + + private static NameMapper selectNameMapper( final Map nameMappers ) + { + 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/named/NoopNameMapper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NoopNameMapper.java deleted file mode 100644 index 3fbedb38a..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NoopNameMapper.java +++ /dev/null @@ -1,49 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext.named; - -/* - * 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.Collections; - -import javax.inject.Named; -import javax.inject.Singleton; - -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.artifact.Artifact; -import org.eclipse.aether.metadata.Metadata; - -/** - * No-op {@link NameMapper}, always returns empty collection. - */ -@Singleton -@Named( NoopNameMapper.NAME ) -public class NoopNameMapper - implements NameMapper -{ - public static final String NAME = "noop"; - - @Override - public Collection nameLocks( final RepositorySystemSession session, - final Collection artifacts, - final Collection metadatas ) - { - return Collections.emptyList(); - } -} 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 + } + } +} From 4b1b096b40b03334ae8a4d3c8e2fdd7e72fee809 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 16:38:55 +0200 Subject: [PATCH 06/10] Update doco --- .../site/markdown/synccontextfactory.md.vm | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm b/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm index 54697a5a5..f5fd26045 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 +Using this module there are several configuration options of `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,10 @@ 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 +${esc.hash}${esc.hash} Provided Implementation 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`. +that uses resolver named locks. ${esc.hash}${esc.hash} Configuration Options for "named" `SyncContextFactory` @@ -65,17 +53,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): From f9cb61886817a2c4fa9a66e7320389ec52d6f795 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 16:48:28 +0200 Subject: [PATCH 07/10] Hash file canonical path to form lock name --- .../impl/DefaultTrackingFileManager.java | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) 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 87e8e8c92..83d56506f 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 @@ -26,6 +26,9 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.RandomAccessFile; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -39,6 +42,7 @@ import org.eclipse.aether.named.NamedLockFactory; import org.eclipse.aether.spi.locator.Service; import org.eclipse.aether.spi.locator.ServiceLocator; +import org.eclipse.aether.util.ChecksumUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -77,7 +81,26 @@ public void initService( final ServiceLocator locator ) private String getFileKey( final File file ) { - return LOCK_PREFIX + "foo"; // hash file abs path? + try + { + Map checksums = ChecksumUtils.calc( + file.getCanonicalPath().getBytes( StandardCharsets.UTF_8 ), + Collections.singletonList( "SHA-1" ) ); + Object checksum = checksums.get( "SHA-1" ); + if ( checksum instanceof RuntimeException ) + { + throw (RuntimeException) checksum; + } + else if ( checksum instanceof Exception ) + { + throw new RuntimeException( ( Exception ) checksum ); + } + return LOCK_PREFIX + checksum; + } + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } } @Override From 0abbe33aeee700fad295b9c8caa16948f1fd8e24 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 18:28:48 +0200 Subject: [PATCH 08/10] Make new NamedFactorySelector Guice managed. It was in SISU (due SISU index) and in ServiceLocator (kinda), but was not in Guice, but it worked, as it has default ctor. Once default ctor (along with ServiceLocator) is gone, these errors will be easier to spot. --- .../main/java/org/eclipse/aether/impl/guice/AetherModule.java | 2 ++ 1 file changed, 2 insertions(+) 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 0fa915cf4..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,6 +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.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; @@ -157,6 +158,7 @@ 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 ) From dac6f97b38ec7e5175fa8e4d01eeae0b01c8608c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 27 Apr 2021 19:22:29 +0200 Subject: [PATCH 09/10] PR comments, typos, tidy --- .../impl/DefaultTrackingFileManager.java | 27 ++----------------- .../DefaultSyncContextFactory.java | 3 +-- .../synccontext/NamedLockFactorySelector.java | 6 ++--- .../site/markdown/synccontextfactory.md.vm | 8 +++--- 4 files changed, 9 insertions(+), 35 deletions(-) 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 83d56506f..30e14c30d 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 @@ -26,9 +26,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.RandomAccessFile; -import java.io.UncheckedIOException; -import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -42,7 +39,6 @@ import org.eclipse.aether.named.NamedLockFactory; import org.eclipse.aether.spi.locator.Service; import org.eclipse.aether.spi.locator.ServiceLocator; -import org.eclipse.aether.util.ChecksumUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,26 +77,7 @@ public void initService( final ServiceLocator locator ) private String getFileKey( final File file ) { - try - { - Map checksums = ChecksumUtils.calc( - file.getCanonicalPath().getBytes( StandardCharsets.UTF_8 ), - Collections.singletonList( "SHA-1" ) ); - Object checksum = checksums.get( "SHA-1" ); - if ( checksum instanceof RuntimeException ) - { - throw (RuntimeException) checksum; - } - else if ( checksum instanceof Exception ) - { - throw new RuntimeException( ( Exception ) checksum ); - } - return LOCK_PREFIX + checksum; - } - catch ( IOException e ) - { - throw new UncheckedIOException( e ); - } + return LOCK_PREFIX + file.getAbsolutePath(); } @Override @@ -228,7 +205,7 @@ public Properties update( File file, Map updates ) } else { - throw new IllegalStateException( "Could not acquire shared lock for: " + file ); + throw new IllegalStateException( "Could not acquire exclusive lock for: " + file ); } } catch ( InterruptedException e ) 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 09c90ab6d..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 @@ -34,8 +34,7 @@ import org.eclipse.aether.spi.synccontext.SyncContextFactory; /** - * Default {@link SyncContextFactory} implementation that uses named locks, but supports "presets" for "global" and - * "nolock" behaviour as well. + * Default {@link SyncContextFactory} implementation that uses named locks. */ @Singleton @Named diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java index 63f69f7c5..190950fc8 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java @@ -37,8 +37,8 @@ import org.eclipse.aether.named.providers.NoopNamedLockFactory; /** - * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentiall - * all the configuration is here. + * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially + * all the named locks configuration is here. */ @Singleton @Named @@ -102,7 +102,7 @@ public NamedLockFactory getSelectedNamedLockFactory() } /** - * Returns the selected {@link NamedLockFactory}, never null. + * Returns the selected {@link NameMapper}, never null. */ public NameMapper getSelectedNameMapper() { diff --git a/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm b/maven-resolver-impl/src/site/markdown/synccontextfactory.md.vm index f5fd26045..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. --> -Using this module there are several configuration options 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,10 +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 Implementation - -This module provides a default implementation for the interface `org.eclipse.aether.spi.synccontext.SyncContextFactory` -that uses resolver named locks. +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` From ce2124726400535f3ad5198bdd163759e84dd9c5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 28 Apr 2021 16:05:27 +0200 Subject: [PATCH 10/10] Align messages --- .../aether/internal/impl/DefaultTrackingFileManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 30e14c30d..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 @@ -122,7 +122,7 @@ public Properties read( File file ) } else { - throw new IllegalStateException( "Could not acquire shared lock for: " + file ); + throw new IllegalStateException( "Could not acquire read lock for: " + file ); } } catch ( InterruptedException e ) @@ -205,7 +205,7 @@ public Properties update( File file, Map updates ) } else { - throw new IllegalStateException( "Could not acquire exclusive lock for: " + file ); + throw new IllegalStateException( "Could not acquire write lock for: " + file ); } } catch ( InterruptedException e )