bugfix: Addressed a critical concurrent error caused by incorrect Jed…

…is usage

pom.xml

@@ -3,8 +3,8 @@
     <modelVersion>4.0.0</modelVersion>
 
     <groupId>site.hellooo</groupId>
-    <artifactId>hellooo-distributedlock</artifactId>
-    <version>0.0.5-GA</version>
+    <artifactId>halo-distributedlock</artifactId>
+    <version>0.0.6-GA</version>
 
     <properties>
         <project.build.sourceEncoding>UTF8</project.build.sourceEncoding>
@@ -21,7 +21,7 @@
             <artifactId>jedis</artifactId>
             <version>${jedis.version}</version>
         </dependency>
-        <!--         test dependencies  -->
+        <!-- test dependencies -->
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
@@ -42,20 +42,20 @@
         </dependency>
     </dependencies>
 
-<!--    发布maven中央仓库所需数据 -->
+    <!-- 发布maven中央仓库所需数据 -->
     <name>distributed-lock</name>
     <description>a simple but reliable distributed lock implementation</description>
-    <url>https://github.com/hellooo-stack/hellooo-distributedlock</url>
+    <url>https://github.com/hellooo-stack/halo-distributedlock</url>
     <licenses>
         <license>
             <name>Apache License, Version 2.0</name>
             <url>https://www.apache.org/licenses/LICENSE-2.0</url>
         </license>
     </licenses>
     <scm>
-        <url>https://github.com/hellooo-stack/hellooo-distributedlock</url>
-        <connection>https://github.com/hellooo-stack/hellooo-distributedlock.git</connection>
-        <developerConnection>https://github.com/hellooo-stack/hellooo-distributedlock.git</developerConnection>
+        <url>https://github.com/hellooo-stack/halo-distributedlock</url>
+        <connection>https://github.com/hellooo-stack/halo-distributedlock.git</connection>
+        <developerConnection>https://github.com/hellooo-stack/halo-distributedlock.git</developerConnection>
         <tag>HEAD</tag>
     </scm>
     <developers>
@@ -66,7 +66,7 @@
     </developers>
     <distributionManagement>
         <snapshotRepository>
-            <!--             对应settings.xml内配置的<server>下的id  -->
+            <!-- 对应settings.xml内配置的<server>下的id -->
             <id>ossrh</id>
             <url>https://s01.oss.sonatype.org/content/repositories/snapshots</url>
         </snapshotRepository>
@@ -80,7 +80,7 @@
             <id>release</id>
             <build>
                 <plugins>
-                    <!--             生成源码 jar  -->
+                    <!-- 生成源码 jar -->
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-source-plugin</artifactId>
@@ -94,7 +94,7 @@
                             </execution>
                         </executions>
                     </plugin>
-                    <!--             生成javadoc jar  -->
+                    <!-- 生成javadoc jar -->
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-javadoc-plugin</artifactId>
@@ -108,7 +108,7 @@
                             </execution>
                         </executions>
                     </plugin>
-                    <!--             gpg 签名  -->
+                    <!-- gpg 签名 -->
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-gpg-plugin</artifactId>
@@ -123,7 +123,7 @@
                             </execution>
                         </executions>
                     </plugin>
-                    <!--             nexus 组件发布  -->
+                    <!-- nexus 组件发布 -->
                     <plugin>
                         <groupId>org.sonatype.plugins</groupId>
                         <artifactId>nexus-staging-maven-plugin</artifactId>

src/main/java/site/hellooo/distributedlock/LockCallback.java

@@ -1,12 +1,12 @@
 package site.hellooo.distributedlock;
 
 public interface LockCallback {
-    //    execute immediately after the lock granted to current thread
+    // execute immediately after the lock granted to current thread
     void afterLocked(LockContext lockContext);
 
-    //    execute before parking the current thread
+    // execute before parking the current thread
     void beforeParking(LockContext lockContext);
 
-    //    execute immediately after the lock released
+    // execute immediately after the lock released
     void afterUnlocked(LockContext lockContext);
 }

src/main/java/site/hellooo/distributedlock/LockContext.java

@@ -8,19 +8,18 @@
  * context of the lock, holding the necessary message for handling the lock operation
  */
 public interface LockContext {
-    String lockTarget();
-    //    the user config of this lock
-    LockOptions lockOptions();
+    String target();
+    // user options of the lock
+    LockOptions options();
 
-    //    thread which is holding the lock in current process
-//    return value should not be null
-//    todo add annotation
+    // thread which is holding the lock in current process
+    // return value should not be null
     AtomicReference<Thread> holdingThread();
 
-    //    lockState of the holding thread in current process
+    // lockState of the holding thread in current process
     AtomicReference<LockState<?>> holdingLockState();
 
-    //    operation handler for dealing with the coordinator
+    // operation handler for dealing with the coordinator
     LockHandler lockHandler();
 
     LockCallback lockCallback();

src/main/java/site/hellooo/distributedlock/LockState.java

@@ -4,13 +4,13 @@
 
 public interface LockState<T> extends Serializable {
 
-    //    get the identifier representing this state
-//    such as a redis key, a zookeeper path, a file name
+    // return the identifier representing this state
+    // such as a redis key, a zookeeper path, a file name, etc.
     String getIdentifier();
 
     void setIdentifier(String identifier);
 
-    //    get the value in the state
+    // return the value in the state
     T getValue();
 
     void setValue(T value);

src/main/java/site/hellooo/distributedlock/common/ProcessUtils.java

@@ -16,7 +16,7 @@ public static String getProcessId() {
 
         }
 
-//        if no process id returned, return the default one
+        // if no process id returned, return the default one
         return DEFAULT_FALLBACK_PROCESS_ID;
     }
 }

src/main/java/site/hellooo/distributedlock/common/StringUtils.java

@@ -10,7 +10,7 @@ public static boolean isEmpty(CharSequence charSequence) {
     }
 
     public static boolean isEmpty(String string) {
-        return string == null || string.length() == 0;
+        return string == null || string.isEmpty();
     }
 
     public static boolean isNotEmpty(CharSequence charSequence) {

src/main/java/site/hellooo/distributedlock/config/LockOptions.java

@@ -7,22 +7,20 @@
 import java.io.Serializable;
 import java.util.StringJoiner;
 
-// 提供用户使用的参数配置类
-// redis专用， 后续优化
+// Maybe a little hardcode for redis, It still needs to do some work if you want to use other coordinators,
+// such as zookeeper, etcd, etc.
 public class LockOptions implements Reusable<LockOptions>, Serializable {
 
-    //    prefix of lock identifier
+    // prefix of lock identifier
     private final String identifierPrefix;
-    //    suffix of lock identifier
+    // suffix of lock identifier
     private final String identifierSuffix;
-    //    milliseconds of retry thread execute interval
+    // milliseconds of retry thread execute interval
     private final long retryIntervalMilliseconds;
-    //    milliseconds of lease thread execute interval, should be smaller than leaseMilliseconds
+    //  milliseconds of lease thread execute interval, should be smaller than leaseMilliseconds
     private final long leaseIntervalMilliseconds;
-    //    milliseconds to lease per time
+    // milliseconds to lease per time
     private final long leaseMilliseconds;
-
-    //    todo
     private final Coordinator coordinator;
 
     private LockOptions(final String identifierPrefix,
@@ -36,6 +34,7 @@ private LockOptions(final String identifierPrefix,
         ArgChecker.check(retryIntervalMilliseconds > 0, "retryIntervalMilliseconds is " + retryIntervalMilliseconds + " (expected > 0).");
         ArgChecker.check(leaseIntervalMilliseconds > 0, "leaseIntervalMilliseconds is " + leaseIntervalMilliseconds + " (expected > 0).");
         ArgChecker.check(leaseMilliseconds > 0, "leaseMilliseconds is " + leaseMilliseconds + " (expected > 0).");
+        ArgChecker.check(leaseMilliseconds > leaseIntervalMilliseconds, "leaseMilliseconds less than leaseIntervalMilliseconds (expected greater than leaseIntervalMilliseconds).");
         ArgChecker.check(coordinator != null, "coordinator is null (expected not null).");
 
         this.identifierPrefix = identifierPrefix;
@@ -99,13 +98,13 @@ public static class LockOptionsBuilder {
         private static final String DEFAULT_IDENTIFIER_PREFIX = "";
         private static final String DEFAULT_IDENTIFIER_SUFFIX = "";
 
-//        默认20毫秒重试一次
+        // 20 milliseconds by default
         private static final long DEFAULT_RETRY_INTERVAL_MILLISECONDS = 10L;
-//        默认1秒续期一次
+        // 1 second a lease by default
         private static final long DEFAULT_LEASE_INTERVAL_MILLISECONDS = 1000L;
-//        默认每次续期都将过期时间设置为5秒
+        // default lease time is 5 seconds
         private static final long DEFAULT_LEASE_MILLISECONDS = 5000L;
-
+        // default coordinator is redis(singleton)
         private static final Coordinator DEFAULT_COORDINATOR = Coordinator.REDIS_SINGLETON;
 
         private String identifierPrefix = DEFAULT_IDENTIFIER_PREFIX;

src/main/java/site/hellooo/distributedlock/enums/Coordinator.java

@@ -1,6 +1,5 @@
 package site.hellooo.distributedlock.enums;
 
-// todo add clone method, some reusable interface may use that method to do the clone
 public enum Coordinator {
 
     REDIS_SINGLETON("redis_singleton"),

src/main/java/site/hellooo/distributedlock/impl/LockStateBuilder.java

@@ -16,9 +16,6 @@ public class LockStateBuilder {
     private Coordinator coordinator = DEFAULT_COORDINATOR;
     private String identifier;
 
-    private Object value;
-
-
     public LockStateBuilder() {
 
     }
@@ -38,23 +35,17 @@ public LockStateBuilder identifier(String identifier) {
         return this;
     }
 
-    public LockStateBuilder value(Object value) {
-        this.value = value;
-        return this;
-    }
-
     public LockState<?> build() {
         ArgChecker.check(!StringUtils.isEmpty(identifier), "identifier is empty (expected not empty).");
-//        ArgChecker.check(value != null, "value is null (expected not null).");
 
         String generatedIdentifier = lockOptions.getIdentifierPrefix() + this.identifier;
         generatedIdentifier = generatedIdentifier + lockOptions.getIdentifierSuffix();
 
         switch (coordinator) {
             case REDIS_SINGLETON:
-                if (this.value != null && !(this.value instanceof String)) {
-                    throw new IllegalArgumentException("Fatal: invalid type of 'value', type is " + value.getClass().getSimpleName());
-                }
+                // if (this.value == null || !(this.value instanceof String)) {
+                //     throw new IllegalArgumentException("Fatal: invalid type of 'value', type is " + (value == null ? "null" : value.getClass().getSimpleName()));
+                // }
 
                 return new RedisLockState(generatedIdentifier);
             case REDIS_CLUSTER:

src/main/java/site/hellooo/distributedlock/impl/ReentrantDistributedLock.java

@@ -34,12 +34,12 @@ public ReentrantDistributedLock(LockOptions lockOptions, String lockTarget, Lock
             private final AtomicReference<LockState<?>> holdingLockState = new AtomicReference<>();
 
             @Override
-            public String lockTarget() {
+            public String target() {
                 return lockTarget;
             }
 
             @Override
-            public LockOptions lockOptions() {
+            public LockOptions options() {
                 return lockOptions;
             }
 
@@ -88,7 +88,7 @@ private Node addWaiter() {
         return currentNode;
     }
 
-//    enqueue node and return the prev node
+    // enqueue node and return the prev node
     private Node enqueue(final Node node) {
 
         while (true) {
@@ -115,12 +115,12 @@ private void acquireQueued(final Node node) {
 
         while (true) {
             final Node prev = node.prev.get();
-//            only if prev node is head, and then we try to get the lock
+            // only if prev node is head, and then we try to get the lock
             if (prev == head.get() && tryLock()) {
-//                if tryLock success, it means that the prev node has release the lock,
-//                so we need to remove it from the queue
+                // if tryLock success, it means that the prev node has release the lock,
+                // so we need to remove it from the queue
                 head.set(node);
-//                set to null, help with gc
+                // set to null, help with gc
                 prev.next.set(null);
                 break;
             }
@@ -158,8 +158,6 @@ public boolean tryLock() {
 
         LockState<?> lockState = new LockStateBuilder(lockOptions)
                 .identifier(lockTarget)
-//                todo: need a value builder for difference state type
-//                .value()
                 .build();
         boolean locked = false;
         try {
@@ -208,11 +206,12 @@ public void unlock() {
         }
         try {
             lockHandler.removeState(holdingLockState, lockContext);
-            lockCallback.afterUnlocked(lockContext);
         } catch (LockStateNotRemovedException ignored) {
-
         } finally {
+            lockCallback.afterUnlocked(lockContext);
+
             holdingThreadReference.compareAndSet(holdingThread, null);
+            holdingLockStateReference.compareAndSet(holdingLockState, null);
             unparkQueueHead();
         }
     }

src/main/java/site/hellooo/distributedlock/impl/ReentrantDistributedLockBuilder.java

@@ -1,7 +1,6 @@
 package site.hellooo.distributedlock.impl;
 
-import redis.clients.jedis.Jedis;
-import site.hellooo.distributedlock.LockCallback;
+import redis.clients.jedis.JedisPool;
 import site.hellooo.distributedlock.LockHandler;
 import site.hellooo.distributedlock.common.StringUtils;
 import site.hellooo.distributedlock.config.LockOptions;
@@ -18,9 +17,8 @@ public class ReentrantDistributedLockBuilder {
     private String lockTarget = null;
 
     private LockHandler lockHandler = null;
-    private LockCallback lockCallback = null;
 
-    private Jedis jedis;
+    private JedisPool jedisPool;
 
     public ReentrantDistributedLockBuilder lockOptions(LockOptions lockOptions) {
         this.lockOptions = lockOptions;
@@ -37,13 +35,8 @@ public ReentrantDistributedLockBuilder lockHandler(LockHandler lockHandler) {
         return this;
     }
 
-    public ReentrantDistributedLockBuilder lockCallback(LockCallback lockCallback) {
-        this.lockCallback = lockCallback;
-        return this;
-    }
-
-    public ReentrantDistributedLockBuilder jedis(Jedis jedis) {
-        this.jedis = jedis;
+    public ReentrantDistributedLockBuilder jedisPool(JedisPool jedisPool) {
+        this.jedisPool = jedisPool;
         return this;
     }
 
@@ -56,17 +49,13 @@ public ReentrantDistributedLock build() {
         Coordinator coordinator = lockOptions.getCoordinator();
         switch (coordinator) {
             case REDIS_SINGLETON:
-                if (this.jedis == null) {
+                if (this.jedisPool == null) {
                     throw new BuilderEssentialFieldNotSetException("Fatal: miss essential component 'jedis'!");
                 }
 
                 if (lockHandler == null) {
-                    lockHandler = new RedisLockHandler(this.jedis);
+                    lockHandler = new RedisLockHandler(this.jedisPool);
                 }
-
-//                if (lockCallback == null) {
-//                    lockCallback = new RedisLockCallback();
-//                }
                 break;
             case REDIS_CLUSTER:
             case ZOOKEEPER: