You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2021/06/13 19:35:25 UTC

[maven-resolver] branch master updated: [MRESOLVER-184] Destroy Redisson semaphores if not used anymore

This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 69fd9f3  [MRESOLVER-184] Destroy Redisson semaphores if not used anymore
69fd9f3 is described below

commit 69fd9f38d75fe5fd62c3a6b8d70b1018b67f7a68
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Sat May 29 21:44:21 2021 +0200

    [MRESOLVER-184] Destroy Redisson semaphores if not used anymore
    
    Drop redundant reference holding of implementations needed for those requiring
    cleanup on destroy. But moreover, makes factory support create/destroy
    symmetrical.
    
    Make two interested factories keep their own map to destroy things.
    
    This closes #111
---
 .../HazelcastSemaphoreNamedLockFactory.java        | 21 +++++----
 .../RedissonReadWriteLockNamedLockFactory.java     | 11 ++---
 .../RedissonSemaphoreNamedLockFactory.java         | 37 +++++++++++----
 .../LocalReadWriteLockNamedLockFactory.java        | 23 ++++------
 .../providers/LocalSemaphoreNamedLockFactory.java  | 53 +++++++++++-----------
 .../named/support/AdaptedSemaphoreNamedLock.java   |  6 ++-
 .../named/support/NamedLockFactorySupport.java     | 28 +++++++-----
 .../aether/named/support/NamedLockSupport.java     |  5 +-
 .../named/support/ReadWriteLockNamedLock.java      |  6 ++-
 9 files changed, 110 insertions(+), 80 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..01450db 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,7 +24,6 @@ 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;
@@ -65,11 +64,14 @@ public class HazelcastSemaphoreNamedLockFactory
     }
 
     @Override
-    protected NamedLockSupport createLock( final String name )
+    protected AdaptedSemaphoreNamedLock createLock( final String name )
     {
-        ISemaphore semaphore = semaphores.computeIfAbsent(
-                name, k -> semaphoreFunction.apply( hazelcastInstance, k )
-        );
+        ISemaphore semaphore = semaphores.computeIfAbsent( name, k ->
+        {
+            ISemaphore result = semaphoreFunction.apply( hazelcastInstance, k );
+            result.init( Integer.MAX_VALUE );
+            return result;
+        } );
         return new AdaptedSemaphoreNamedLock( name, this, new HazelcastSemaphore( semaphore ) );
     }
 
@@ -83,11 +85,15 @@ public class HazelcastSemaphoreNamedLockFactory
     }
 
     @Override
-    protected void destroyLock( final NamedLockSupport lock )
+    protected void destroyLock( final String name )
     {
-        ISemaphore semaphore = semaphores.remove( lock.name() );
+        ISemaphore semaphore = semaphores.remove( name );
         if ( destroySemaphore )
         {
+            if ( semaphore == null )
+            {
+                throw new IllegalStateException( "Semaphore expected but does not exist: " + name );
+            }
             semaphore.destroy();
         }
     }
@@ -98,7 +104,6 @@ public class HazelcastSemaphoreNamedLockFactory
 
         private HazelcastSemaphore( final ISemaphore semaphore )
         {
-            semaphore.init( Integer.MAX_VALUE );
             this.semaphore = semaphore;
         }
 
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..647f775 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;
@@ -36,12 +36,9 @@ public class RedissonReadWriteLockNamedLockFactory
     public static final String NAME = "rwlock-redisson";
 
     @Override
-    protected NamedLockSupport createLock( final String name )
+    protected ReadWriteLockNamedLock createLock( final String name )
     {
-        return new ReadWriteLockNamedLock(
-                   name,
-                   this,
-                   redissonClient.getReadWriteLock( NAME_PREFIX + name )
-        );
+        RReadWriteLock readWriteLock = redissonClient.getReadWriteLock( NAME_PREFIX + name );
+        return 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..73c538a 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,11 +20,12 @@ 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;
 import javax.inject.Singleton;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -37,12 +38,34 @@ public class RedissonSemaphoreNamedLockFactory
 {
     public static final String NAME = "semaphore-redisson";
 
+    private final ConcurrentMap<String, RSemaphore> semaphores;
+
+    public RedissonSemaphoreNamedLockFactory()
+    {
+        this.semaphores = new ConcurrentHashMap<>();
+    }
+
     @Override
-    protected NamedLockSupport createLock( final String name )
+    protected AdaptedSemaphoreNamedLock createLock( final String name )
     {
-        return new AdaptedSemaphoreNamedLock(
-                   name, this, new RedissonSemaphore( redissonClient.getSemaphore( NAME_PREFIX + name ) )
-    );
+        RSemaphore semaphore = semaphores.computeIfAbsent( name, k ->
+        {
+            RSemaphore result = redissonClient.getSemaphore( NAME_PREFIX + k );
+            result.trySetPermits( Integer.MAX_VALUE );
+            return result;
+        } );
+        return new AdaptedSemaphoreNamedLock( name, this, new RedissonSemaphore( semaphore ) );
+    }
+
+    @Override
+    protected void destroyLock( final String name )
+    {
+        RSemaphore semaphore = semaphores.remove( name );
+        if ( semaphore == null )
+        {
+            throw new IllegalStateException( "Semaphore expected but does not exist: " + name );
+        }
+        semaphore.delete();
     }
 
     private static final class RedissonSemaphore implements AdaptedSemaphoreNamedLock.AdaptedSemaphore
@@ -51,13 +74,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..34b242a 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,13 +19,12 @@ 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.
@@ -35,15 +34,11 @@ import org.eclipse.aether.named.support.NamedLockFactorySupport;
 public class LocalReadWriteLockNamedLockFactory
     extends NamedLockFactorySupport
 {
-  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 ReadWriteLockNamedLock createLock( final String name )
+    {
+        return new ReadWriteLockNamedLock( name, this, new ReentrantReadWriteLock() );
+    }
 }
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..3be7f4e 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
@@ -36,37 +36,38 @@ import org.eclipse.aether.named.support.NamedLockFactorySupport;
 public class LocalSemaphoreNamedLockFactory
     extends NamedLockFactorySupport
 {
-  public static final String NAME = "semaphore-local";
-
-  @Override
-  protected AdaptedSemaphoreNamedLock createLock( final String name )
-  {
-    return new AdaptedSemaphoreNamedLock( name, this, new JVMSemaphore() );
-  }
-
-  /**
-   * Adapted JVM {@link java.util.concurrent.Semaphore}.
-   */
-  private static final class JVMSemaphore
-      implements AdaptedSemaphoreNamedLock.AdaptedSemaphore
-  {
-    private final Semaphore semaphore;
-
-    private JVMSemaphore()
-    {
-      this.semaphore = new Semaphore( Integer.MAX_VALUE );
-    }
+    public static final String NAME = "semaphore-local";
 
     @Override
-    public boolean tryAcquire( final int perms, final long time, final TimeUnit unit ) throws InterruptedException
+    protected AdaptedSemaphoreNamedLock createLock( final String name )
     {
-      return semaphore.tryAcquire( perms, time, unit );
+      Semaphore semaphore = new Semaphore( Integer.MAX_VALUE );
+      return new AdaptedSemaphoreNamedLock( name, this, new JVMSemaphore( semaphore ) );
     }
 
-    @Override
-    public void release( final int perms )
+    /**
+     * Adapted JVM {@link java.util.concurrent.Semaphore}.
+     */
+    private static final class JVMSemaphore
+        implements AdaptedSemaphoreNamedLock.AdaptedSemaphore
     {
-      semaphore.release( perms );
+        private final Semaphore semaphore;
+
+        private JVMSemaphore( final Semaphore semaphore )
+        {
+          this.semaphore = semaphore;
+        }
+
+        @Override
+        public boolean tryAcquire( final int perms, final long time, final TimeUnit unit ) throws InterruptedException
+        {
+             return semaphore.tryAcquire( perms, time, unit );
+        }
+
+        @Override
+        public void release( final int perms )
+        {
+            semaphore.release( perms );
+        }
     }
-  }
 }
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..483989e 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
@@ -27,7 +27,8 @@ import java.util.concurrent.TimeUnit;
  * Named lock support implementation that is using "adapted" semaphore (to be able to use semaphores not sharing common
  * API).
  */
-public class AdaptedSemaphoreNamedLock extends NamedLockSupport
+public class AdaptedSemaphoreNamedLock
+    extends NamedLockSupport
 {
     /**
      * Wrapper for semaphore-like stuff, that do not share common ancestor. Semaphore must be created to support {@link
@@ -61,7 +62,8 @@ 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..9b9a2b4 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,15 +23,16 @@ 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.
  */
-public abstract class NamedLockFactorySupport implements NamedLockFactory
+public abstract class NamedLockFactorySupport
+    implements NamedLockFactory
 {
     protected final Logger logger = LoggerFactory.getLogger( getClass() );
 
@@ -62,20 +63,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( v.namedLock.name() );
                 return null;
             }
             return v;
         } );
-        return destroyed.get();
     }
 
 
@@ -96,9 +94,17 @@ public abstract class NamedLockFactorySupport implements NamedLockFactory
         }
     }
 
+    /**
+     * Implementations shall create and return {@link NamedLockSupport} for given {@code name}, this method must never
+     * return {@code null}.
+     */
     protected abstract NamedLockSupport createLock( final String name );
 
-    protected void destroyLock( final NamedLockSupport lock )
+    /**
+     * Implementation may override this (empty) method to perform some sort of implementation specific cleanup for
+     * given lock name. Invoked when reference count for given name drops to zero and named lock was removed.
+     */
+    protected void destroyLock( final String name )
     {
         // override if needed
     }
@@ -109,9 +115,9 @@ public abstract class NamedLockFactorySupport implements NamedLockFactory
 
         private final AtomicInteger referenceCount;
 
-        private NamedLockHolder( NamedLockSupport namedLock )
+        private NamedLockHolder( final NamedLockSupport namedLock )
         {
-            this.namedLock = namedLock;
+            this.namedLock = Objects.requireNonNull( namedLock );
             this.referenceCount = new AtomicInteger( 0 );
         }
 
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..38915b2 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
@@ -26,7 +26,8 @@ import org.slf4j.LoggerFactory;
 /**
  * Support class for {@link NamedLock} implementations providing reference counting.
  */
-public abstract class NamedLockSupport implements NamedLock
+public abstract class NamedLockSupport
+    implements NamedLock
 {
     protected final Logger logger = LoggerFactory.getLogger( getClass() );
 
@@ -49,6 +50,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..2557ed8 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
@@ -29,7 +29,8 @@ import java.util.concurrent.locks.ReadWriteLock;
  * reentrancy, non re-entrant locks will NOT work. It is the responsibility of an adapting lock, to ensure that
  * above lock requirement stands.
  */
-public class ReadWriteLockNamedLock extends NamedLockSupport
+public class ReadWriteLockNamedLock
+    extends NamedLockSupport
 {
     private enum Step
     {
@@ -48,7 +49,8 @@ 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 );