This is an automated email from the ASF dual-hosted git repository. cstamas pushed a commit to branch MRESOLVER-184-alternate-cleaner in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit 5117266bcc43e04d4ced5868b20ebe250c5ceefd Author: Tamas Cservenak <[email protected]> AuthorDate: Sat May 29 21:44:21 2021 +0200 [MRESOLVER-184] Cleaner destroy This PR drops redundant reference holding of implementations needed for those requiring cleanup on destroy. But moreover, this PR also contains changes making factory support create/destroy symmetrical. --- .../HazelcastSemaphoreNamedLockFactory.java | 22 ++++----- .../redisson/RedissonNamedLockFactorySupport.java | 6 ++- .../RedissonReadWriteLockNamedLockFactory.java | 14 +++--- .../RedissonSemaphoreNamedLockFactory.java | 23 ++++++---- .../LocalReadWriteLockNamedLockFactory.java | 29 ++++++------ .../providers/LocalSemaphoreNamedLockFactory.java | 13 ++++-- .../named/providers/NoopNamedLockFactory.java | 10 ++-- .../named/support/AdaptedSemaphoreNamedLock.java | 2 +- .../named/support/NamedLockFactorySupport.java | 53 +++++++++++++++------- .../aether/named/support/NamedLockSupport.java | 6 +-- .../named/support/ReadWriteLockNamedLock.java | 2 +- 11 files changed, 101 insertions(+), 79 deletions(-) diff --git a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java index cd2336b..c0ee9a3 100644 --- a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java +++ b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java @@ -24,10 +24,7 @@ import com.hazelcast.cp.ISemaphore; import org.eclipse.aether.named.support.AdaptedSemaphoreNamedLock; import org.eclipse.aether.named.support.AdaptedSemaphoreNamedLock.AdaptedSemaphore; import org.eclipse.aether.named.support.NamedLockFactorySupport; -import org.eclipse.aether.named.support.NamedLockSupport; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; @@ -36,7 +33,7 @@ import java.util.function.BiFunction; * use {@link HazelcastInstance} backed by Hazelcast Server or Hazelcast Client. */ public class HazelcastSemaphoreNamedLockFactory - extends NamedLockFactorySupport + extends NamedLockFactorySupport<ISemaphore> { protected static final String NAME_PREFIX = "maven:resolver:"; @@ -48,8 +45,6 @@ public class HazelcastSemaphoreNamedLockFactory private final boolean manageHazelcast; - private final ConcurrentMap<String, ISemaphore> semaphores; - public HazelcastSemaphoreNamedLockFactory( final HazelcastInstance hazelcastInstance, final BiFunction<HazelcastInstance, String, ISemaphore> semaphoreFunction, @@ -61,16 +56,16 @@ public class HazelcastSemaphoreNamedLockFactory this.semaphoreFunction = semaphoreFunction; this.destroySemaphore = destroySemaphore; this.manageHazelcast = manageHazelcast; - this.semaphores = new ConcurrentHashMap<>(); } @Override - protected NamedLockSupport createLock( final String name ) + protected NamedLockHolder<ISemaphore> createLock( final String name ) { - ISemaphore semaphore = semaphores.computeIfAbsent( - name, k -> semaphoreFunction.apply( hazelcastInstance, k ) + ISemaphore semaphore = semaphoreFunction.apply( hazelcastInstance, name ); + return new NamedLockHolder<>( + semaphore, + new AdaptedSemaphoreNamedLock( name, this, new HazelcastSemaphore( semaphore ) ) ); - return new AdaptedSemaphoreNamedLock( name, this, new HazelcastSemaphore( semaphore ) ); } @Override @@ -83,12 +78,11 @@ public class HazelcastSemaphoreNamedLockFactory } @Override - protected void destroyLock( final NamedLockSupport lock ) + protected void destroyLock( final String name, final NamedLockHolder<ISemaphore> holder ) { - ISemaphore semaphore = semaphores.remove( lock.name() ); if ( destroySemaphore ) { - semaphore.destroy(); + holder.getImplementation().destroy(); } } diff --git a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java index d49c19e..c0273af 100644 --- a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java +++ b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java @@ -32,9 +32,11 @@ import java.nio.file.Paths; /** * Support class for factories using {@link RedissonClient}. + * + * @param <I> The type of redisson object backing named lock. */ -public abstract class RedissonNamedLockFactorySupport - extends NamedLockFactorySupport +public abstract class RedissonNamedLockFactorySupport<I> + extends NamedLockFactorySupport<I> { protected static final String NAME_PREFIX = "maven:resolver:"; diff --git a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonReadWriteLockNamedLockFactory.java b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonReadWriteLockNamedLockFactory.java index 089ecc9..9bce176 100644 --- a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonReadWriteLockNamedLockFactory.java +++ b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonReadWriteLockNamedLockFactory.java @@ -20,7 +20,7 @@ package org.eclipse.aether.named.redisson; */ import org.eclipse.aether.named.support.ReadWriteLockNamedLock; -import org.eclipse.aether.named.support.NamedLockSupport; +import org.redisson.api.RReadWriteLock; import javax.inject.Named; import javax.inject.Singleton; @@ -31,17 +31,17 @@ import javax.inject.Singleton; @Singleton @Named( RedissonReadWriteLockNamedLockFactory.NAME ) public class RedissonReadWriteLockNamedLockFactory - extends RedissonNamedLockFactorySupport + extends RedissonNamedLockFactorySupport<RReadWriteLock> { public static final String NAME = "rwlock-redisson"; @Override - protected NamedLockSupport createLock( final String name ) + protected NamedLockHolder<RReadWriteLock> createLock( final String name ) { - return new ReadWriteLockNamedLock( - name, - this, - redissonClient.getReadWriteLock( NAME_PREFIX + name ) + RReadWriteLock readWriteLock = redissonClient.getReadWriteLock( NAME_PREFIX + name ); + return new NamedLockHolder<>( + readWriteLock, + new ReadWriteLockNamedLock( name, this, readWriteLock ) ); } } diff --git a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonSemaphoreNamedLockFactory.java b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonSemaphoreNamedLockFactory.java index bef84cc..1e0bbec 100644 --- a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonSemaphoreNamedLockFactory.java +++ b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonSemaphoreNamedLockFactory.java @@ -20,7 +20,6 @@ package org.eclipse.aether.named.redisson; */ import org.eclipse.aether.named.support.AdaptedSemaphoreNamedLock; -import org.eclipse.aether.named.support.NamedLockSupport; import org.redisson.api.RSemaphore; import javax.inject.Named; @@ -32,17 +31,23 @@ import java.util.concurrent.TimeUnit; */ @Singleton @Named( RedissonSemaphoreNamedLockFactory.NAME ) -public class RedissonSemaphoreNamedLockFactory - extends RedissonNamedLockFactorySupport +public class RedissonSemaphoreNamedLockFactory extends RedissonNamedLockFactorySupport<RSemaphore> { public static final String NAME = "semaphore-redisson"; @Override - protected NamedLockSupport createLock( final String name ) + protected NamedLockHolder<RSemaphore> createLock( final String name ) { - return new AdaptedSemaphoreNamedLock( - name, this, new RedissonSemaphore( redissonClient.getSemaphore( NAME_PREFIX + name ) ) - ); + RSemaphore semaphore = redissonClient.getSemaphore( NAME_PREFIX + name ); + semaphore.trySetPermits( Integer.MAX_VALUE ); + return new NamedLockHolder<>( semaphore, new AdaptedSemaphoreNamedLock( name, this, + new RedissonSemaphore( redissonClient.getSemaphore( NAME_PREFIX + name ) ) ) ); + } + + @Override + protected void destroyLock( String name, NamedLockHolder<RSemaphore> holder ) + { + holder.getImplementation().delete(); } private static final class RedissonSemaphore implements AdaptedSemaphoreNamedLock.AdaptedSemaphore @@ -51,13 +56,11 @@ public class RedissonSemaphoreNamedLockFactory private RedissonSemaphore( final RSemaphore semaphore ) { - semaphore.trySetPermits( Integer.MAX_VALUE ); this.semaphore = semaphore; } @Override - public boolean tryAcquire( final int perms, final long time, final TimeUnit unit ) - throws InterruptedException + public boolean tryAcquire( final int perms, final long time, final TimeUnit unit ) throws InterruptedException { return semaphore.tryAcquire( perms, time, unit ); } diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalReadWriteLockNamedLockFactory.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalReadWriteLockNamedLockFactory.java index 316a8d2..1a7e2c1 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalReadWriteLockNamedLockFactory.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalReadWriteLockNamedLockFactory.java @@ -19,31 +19,28 @@ package org.eclipse.aether.named.providers; * under the License. */ -import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.eclipse.aether.named.support.NamedLockFactorySupport; +import org.eclipse.aether.named.support.ReadWriteLockNamedLock; import javax.inject.Named; import javax.inject.Singleton; - -import org.eclipse.aether.named.support.ReadWriteLockNamedLock; -import org.eclipse.aether.named.support.NamedLockFactorySupport; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * A JVM-local named lock factory that uses named {@link ReentrantReadWriteLock}s. */ @Singleton @Named( LocalReadWriteLockNamedLockFactory.NAME ) -public class LocalReadWriteLockNamedLockFactory - extends NamedLockFactorySupport +public class LocalReadWriteLockNamedLockFactory extends NamedLockFactorySupport<ReentrantReadWriteLock> { - public static final String NAME = "rwlock-local"; + public static final String NAME = "rwlock-local"; - @Override - protected ReadWriteLockNamedLock createLock( final String name ) - { - return new ReadWriteLockNamedLock( - name, - this, - new ReentrantReadWriteLock() - ); - } + @Override + protected NamedLockHolder<ReentrantReadWriteLock> createLock( final String name ) + { + ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); + return new NamedLockHolder<>( + null, new ReadWriteLockNamedLock( name, this, rwLock ) + ); + } } diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalSemaphoreNamedLockFactory.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalSemaphoreNamedLockFactory.java index 464a432..f84473c 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalSemaphoreNamedLockFactory.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/LocalSemaphoreNamedLockFactory.java @@ -34,14 +34,17 @@ import org.eclipse.aether.named.support.NamedLockFactorySupport; @Singleton @Named( LocalSemaphoreNamedLockFactory.NAME ) public class LocalSemaphoreNamedLockFactory - extends NamedLockFactorySupport + extends NamedLockFactorySupport<Semaphore> { public static final String NAME = "semaphore-local"; @Override - protected AdaptedSemaphoreNamedLock createLock( final String name ) + protected NamedLockHolder<Semaphore> createLock( final String name ) { - return new AdaptedSemaphoreNamedLock( name, this, new JVMSemaphore() ); + Semaphore semaphore = new Semaphore( Integer.MAX_VALUE ); + return new NamedLockHolder<>( + null, new AdaptedSemaphoreNamedLock( name, this, new JVMSemaphore( semaphore ) ) + ); } /** @@ -52,9 +55,9 @@ public class LocalSemaphoreNamedLockFactory { private final Semaphore semaphore; - private JVMSemaphore() + private JVMSemaphore( final Semaphore semaphore ) { - this.semaphore = new Semaphore( Integer.MAX_VALUE ); + this.semaphore = semaphore; } @Override 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 index 66b45ed..cb1fb4a 100644 --- 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 @@ -33,19 +33,21 @@ import org.eclipse.aether.named.support.NamedLockSupport; @Singleton @Named( NoopNamedLockFactory.NAME ) public class NoopNamedLockFactory - extends NamedLockFactorySupport + extends NamedLockFactorySupport<Void> { public static final String NAME = "noop"; @Override - protected NoopNamedLock createLock( final String name ) + protected NamedLockHolder<Void> createLock( final String name ) { - return new NoopNamedLock( name, this ); + return new NamedLockHolder<>( + null, new NoopNamedLock( name, this ) + ); } private static final class NoopNamedLock extends NamedLockSupport { - private NoopNamedLock( final String name, final NamedLockFactorySupport factory ) + private NoopNamedLock( final String name, final NamedLockFactorySupport<?> factory ) { super( name, factory ); } diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java index de12557..d43b87f 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java @@ -61,7 +61,7 @@ public class AdaptedSemaphoreNamedLock extends NamedLockSupport private final AdaptedSemaphore semaphore; - public AdaptedSemaphoreNamedLock( final String name, final NamedLockFactorySupport factory, + public AdaptedSemaphoreNamedLock( final String name, final NamedLockFactorySupport<?> factory, final AdaptedSemaphore semaphore ) { super( name, factory ); diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java index ef5a402..0444220 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java @@ -23,19 +23,21 @@ import org.eclipse.aether.named.NamedLockFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; /** * Support class for {@link NamedLockFactory} implementations providing reference counting. + * + * @param <I> the backing implementation type. */ -public abstract class NamedLockFactorySupport implements NamedLockFactory +public abstract class NamedLockFactorySupport<I> implements NamedLockFactory { protected final Logger logger = LoggerFactory.getLogger( getClass() ); - private final ConcurrentMap<String, NamedLockHolder> locks; + private final ConcurrentMap<String, NamedLockHolder<I>> locks; public NamedLockFactorySupport() { @@ -49,7 +51,7 @@ public abstract class NamedLockFactorySupport implements NamedLockFactory { if ( v == null ) { - v = new NamedLockHolder( createLock( k ) ); + v = createLock( k ); } v.incRef(); return v; @@ -62,20 +64,17 @@ public abstract class NamedLockFactorySupport implements NamedLockFactory // override if needed } - public boolean closeLock( final NamedLockSupport lock ) + public void closeLock( final String name ) { - AtomicBoolean destroyed = new AtomicBoolean( false ); - locks.compute( lock.name(), ( k, v ) -> + locks.compute( name, ( k, v ) -> { if ( v != null && v.decRef() == 0 ) { - destroyLock( v.namedLock ); - destroyed.set( true ); + destroyLock( k, v ); return null; } return v; } ); - return destroyed.get(); } @@ -96,25 +95,47 @@ public abstract class NamedLockFactorySupport implements NamedLockFactory } } - protected abstract NamedLockSupport createLock( final String name ); - - protected void destroyLock( final NamedLockSupport lock ) + /** + * Implementation should create and return {@link NamedLockSupport} for given {@code name}, this method should never + * return {@code null}. + */ + protected abstract NamedLockHolder<I> createLock( final String name ); + + /** + * Implementation may override this (empty) method to perform some sort of implementation specific clean-up for + * given name and holder. Invoked when reference count for holder returned by {@link #createLock(String)} drops to + * zero. + */ + protected void destroyLock( final String name, final NamedLockHolder<I> holder ) { // override if needed } - private static final class NamedLockHolder + /** + * This class is a "holder" for backing implementation (if needed), named lock and reference count. + * + * @param <I> + */ + protected static final class NamedLockHolder<I> { + private final I implementation; + private final NamedLockSupport namedLock; private final AtomicInteger referenceCount; - private NamedLockHolder( NamedLockSupport namedLock ) + public NamedLockHolder( final I implementation, final NamedLockSupport namedLock ) { - this.namedLock = namedLock; + this.implementation = implementation; + this.namedLock = Objects.requireNonNull( namedLock ); this.referenceCount = new AtomicInteger( 0 ); } + public I getImplementation() + { + return implementation; + } + private int incRef() { return referenceCount.incrementAndGet(); diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java index 02f9960..07d72d1 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java @@ -32,9 +32,9 @@ public abstract class NamedLockSupport implements NamedLock private final String name; - private final NamedLockFactorySupport factory; + private final NamedLockFactorySupport<?> factory; - public NamedLockSupport( final String name, final NamedLockFactorySupport factory ) + public NamedLockSupport( final String name, final NamedLockFactorySupport<?> factory ) { this.name = name; this.factory = factory; @@ -49,6 +49,6 @@ public abstract class NamedLockSupport implements NamedLock @Override public void close() { - factory.closeLock( this ); + factory.closeLock( name ); } } diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java index 6344294..58d50a8 100644 --- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java +++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java @@ -48,7 +48,7 @@ public class ReadWriteLockNamedLock extends NamedLockSupport private final ReadWriteLock readWriteLock; - public ReadWriteLockNamedLock( final String name, final NamedLockFactorySupport factory, + public ReadWriteLockNamedLock( final String name, final NamedLockFactorySupport<?> factory, final ReadWriteLock readWriteLock ) { super( name, factory );
