(gravitino) branch main updated: [#8200] improvement(core): throw exception when unlocking without an active lock in TreeLock (#8244)

[jshao]

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

jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git


The following commit(s) were added to refs/heads/main by this push:
     new a4d0545b54 [#8200] improvement(core): throw exception when unlocking 
without an active lock in TreeLock (#8244)
a4d0545b54 is described below

commit a4d0545b54e44504ce445de3f97c218aaf233eb1
Author: keepConcentration <phm...@gmail.com>
AuthorDate: Tue Aug 26 15:03:52 2025 +0900

    [#8200] improvement(core): throw exception when unlocking without an active 
lock in TreeLock (#8244)
    
    ### What changes were proposed in this pull request?
    
    Added a check in `TreeLock.unlock()` to throw `IllegalStateException`
    when `unlock()` is called without any active lock.
    
    ### Why are the changes needed?
    
    Previously, calling `unlock()` without holding a lock (e.g., double
    unlock) was silently ignored, which could hide logic errors in lock
    handling.
    
    Fix: #8200
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes.
    1. Users calling `unlock()` without an active lock will now receive an
    `IllegalStateException`.
    
    ### How was this patch tested?
    
    1. Added a unit test `testDoubleUnlockThrows()` verifying that a second
    `unlock()` call throws `IllegalStateException`
      2. Executed existing unit tests
    
    Co-authored-by: Mini Yu <y...@datastrato.com>
---
 core/src/main/java/org/apache/gravitino/lock/TreeLock.java     | 3 +++
 core/src/test/java/org/apache/gravitino/lock/TestTreeLock.java | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/core/src/main/java/org/apache/gravitino/lock/TreeLock.java 
b/core/src/main/java/org/apache/gravitino/lock/TreeLock.java
index d08026d33a..9ad2565dd3 100644
--- a/core/src/main/java/org/apache/gravitino/lock/TreeLock.java
+++ b/core/src/main/java/org/apache/gravitino/lock/TreeLock.java
@@ -143,6 +143,9 @@ public class TreeLock {
     if (lockType == null) {
       throw new IllegalStateException("We must lock the tree lock before 
unlock it.");
     }
+    if (heldLocks.isEmpty()) {
+      throw new IllegalStateException("We must hold a lock before unlocking 
it.");
+    }
 
     while (!heldLocks.isEmpty()) {
       Pair<TreeLockNode, LockType> pair = heldLocks.pop();
diff --git a/core/src/test/java/org/apache/gravitino/lock/TestTreeLock.java 
b/core/src/test/java/org/apache/gravitino/lock/TestTreeLock.java
index b6a525da78..2ff7c4e8e4 100644
--- a/core/src/test/java/org/apache/gravitino/lock/TestTreeLock.java
+++ b/core/src/test/java/org/apache/gravitino/lock/TestTreeLock.java
@@ -102,4 +102,11 @@ class TestTreeLock {
     Mockito.verify(mockNode2, Mockito.never()).unlock(Mockito.any());
     Mockito.verify(mockNode3, Mockito.never()).unlock(Mockito.any());
   }
+
+  @Test
+  void testDoubleUnlockThrows() {
+    lock.lock(LockType.READ);
+    lock.unlock();
+    assertThrows(IllegalStateException.class, () -> lock.unlock());
+  }
 }