From 3363c664681b8b58238d54776671428fbaac3365 Mon Sep 17 00:00:00 2001
From: YueZhang <69956021+zhangyue19921010@users.noreply.github.com>
Date: Wed, 16 Feb 2022 14:41:25 +0800
Subject: [PATCH] [HUDI-3394] Check isWriteLockedByCurrentThread before unlock
 for InProcessLockProvider (#4819)

Co-authored-by: yuezhang <yuezhang@freewheel.tv>
Co-authored-by: Y Ethan Guo <ethan.guoyihua@gmail.com>
---
 .../client/transaction/lock/InProcessLockProvider.java   | 9 +++++++--
 .../apache/hudi/client/transaction/lock/LockManager.java | 4 ++++
 .../client/transaction/TestInProcessLockProvider.java    | 4 ++--
 .../hudi/client/transaction/TestTransactionManager.java  | 2 +-
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/InProcessLockProvider.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/InProcessLockProvider.java
index 426d1cfaf..9e5a2379c 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/InProcessLockProvider.java
+++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/InProcessLockProvider.java
@@ -19,13 +19,14 @@
 
 package org.apache.hudi.client.transaction.lock;
 
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hudi.common.config.LockConfiguration;
 import org.apache.hudi.common.config.TypedProperties;
 import org.apache.hudi.common.lock.LockProvider;
 import org.apache.hudi.common.lock.LockState;
 import org.apache.hudi.common.util.StringUtils;
 import org.apache.hudi.exception.HoodieLockException;
+
+import org.apache.hadoop.conf.Configuration;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
@@ -92,7 +93,11 @@ public class InProcessLockProvider implements LockProvider<ReentrantReadWriteLoc
   public void unlock() {
     LOG.info(getLogMessage(LockState.RELEASING));
     try {
-      LOCK.writeLock().unlock();
+      if (LOCK.isWriteLockedByCurrentThread()) {
+        LOCK.writeLock().unlock();
+      } else {
+        LOG.warn("Cannot unlock because the current thread does not hold the lock.");
+      }
     } catch (Exception e) {
       throw new HoodieLockException(getLogMessage(LockState.FAILED_TO_RELEASE), e);
     }
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/LockManager.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/LockManager.java
index 773685980..913736cad 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/LockManager.java
+++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/LockManager.java
@@ -83,6 +83,10 @@ public class LockManager implements Serializable, AutoCloseable {
     }
   }
 
+  /**
+   * We need to take care of the scenarios that current thread may not be the holder of this lock
+   * and tries to call unlock()
+   */
   public void unlock() {
     if (writeConfig.getWriteConcurrencyMode().supportsOptimisticConcurrencyControl()) {
       getLockProvider().unlock();
diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java
index 9e7472c13..6c2457874 100644
--- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java
+++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java
@@ -238,7 +238,7 @@ public class TestInProcessLockProvider {
     assertDoesNotThrow(() -> {
       inProcessLockProvider.unlock();
     });
-    assertThrows(HoodieLockException.class, () -> {
+    assertDoesNotThrow(() -> {
       inProcessLockProvider.unlock();
     });
   }
@@ -246,7 +246,7 @@ public class TestInProcessLockProvider {
   @Test
   public void testUnlockWithoutLock() {
     InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
-    assertThrows(HoodieLockException.class, () -> {
+    assertDoesNotThrow(() -> {
       inProcessLockProvider.unlock();
     });
   }
diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestTransactionManager.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestTransactionManager.java
index a1a7f6a31..22f801784 100644
--- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestTransactionManager.java
+++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestTransactionManager.java
@@ -81,7 +81,7 @@ public class TestTransactionManager extends HoodieCommonTestHarness {
     });
 
     transactionManager.endTransaction();
-    assertThrows(HoodieLockException.class, () -> {
+    assertDoesNotThrow(() -> {
       transactionManager.endTransaction();
     });
   }