From 6ec136a03db9f052775c7bd0cbdbd43c696295c9 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Mon, 23 Aug 2021 10:05:38 +0200 Subject: [PATCH] Make sure lock is free'd even on error (fixes #3116) --- .../core/synchronization/LockRepository.java | 8 ++-- .../synchronization/LockRepositoryTest.java | 40 +++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 Core/src/test/java/com/plotsquared/core/synchronization/LockRepositoryTest.java diff --git a/Core/src/main/java/com/plotsquared/core/synchronization/LockRepository.java b/Core/src/main/java/com/plotsquared/core/synchronization/LockRepository.java index 5829ea107..12b63d3b2 100644 --- a/Core/src/main/java/com/plotsquared/core/synchronization/LockRepository.java +++ b/Core/src/main/java/com/plotsquared/core/synchronization/LockRepository.java @@ -71,11 +71,9 @@ public final class LockRepository { * @param runnable Action to run when the lock is available */ public void useLock(final @NonNull LockKey key, final @NonNull Runnable runnable) { - this.useLock(key, lock -> { - lock.lock(); - runnable.run(); - lock.unlock(); - }); + try (LockAccess ignored = lock(key)) { + runnable.run(); + } } /** diff --git a/Core/src/test/java/com/plotsquared/core/synchronization/LockRepositoryTest.java b/Core/src/test/java/com/plotsquared/core/synchronization/LockRepositoryTest.java new file mode 100644 index 000000000..b91ba96e4 --- /dev/null +++ b/Core/src/test/java/com/plotsquared/core/synchronization/LockRepositoryTest.java @@ -0,0 +1,40 @@ +package com.plotsquared.core.synchronization; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class LockRepositoryTest { + + private LockKey key; + private LockRepository lockRepository; + + @BeforeEach + void setUp() { + this.key = LockKey.of("test"); + this.lockRepository = new LockRepository(); + } + + @Test + @DisplayName("Unlock even if there is an error") + void useLockUnlock() { + Lock l = this.lockRepository.getLock(this.key); + // Striped uses a ReentrantLock internally, and we need its isLocked method for this test + if (!(l instanceof ReentrantLock lock)) { + throw new IllegalStateException("Expected a ReentrantLock"); + } + + assertThrows(IllegalStateException.class, () -> { + this.lockRepository.useLock(this.key, () -> { + throw new IllegalStateException(); + }); + }); + assertFalse(lock.isLocked()); + } +}