diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LockInterceptor.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LockInterceptor.java index 46f94cb31cbd6..f454f9feab243 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LockInterceptor.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LockInterceptor.java @@ -2,9 +2,7 @@ import static jakarta.interceptor.Interceptor.Priority.PLATFORM_BEFORE; -import java.lang.annotation.Annotation; -import java.util.Set; -import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import jakarta.annotation.Priority; @@ -21,10 +19,13 @@ @Priority(PLATFORM_BEFORE) public class LockInterceptor { - private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock(); + + // This lock is used exclusively to synchronize the block where we release all read locks and aquire the write lock + private final ReentrantLock rl = new ReentrantLock(); @AroundInvoke - Object lock(InvocationContext ctx) throws Exception { + Object lock(ArcInvocationContext ctx) throws Exception { Lock lock = getLock(ctx); switch (lock.value()) { case WRITE: @@ -33,27 +34,47 @@ Object lock(InvocationContext ctx) throws Exception { return readLock(lock, ctx); case NONE: return ctx.proceed(); + default: + throw new LockException("Unsupported @Lock type found on business method " + ctx.getMethod()); } - throw new LockException("Unsupported @Lock type found on business method " + ctx.getMethod()); } private Object writeLock(Lock lock, InvocationContext ctx) throws Exception { - boolean locked = false; long time = lock.time(); + int readHoldCount = rwl.getReadHoldCount(); + boolean locked = false; + try { - if (time > 0) { - locked = readWriteLock.writeLock().tryLock(time, lock.unit()); - if (!locked) { - throw new LockException("Write lock not acquired in " + lock.unit().toMillis(time) + " ms"); + rl.lock(); + try { + if (readHoldCount > 0) { + // Release all read locks hold by the current thread before acquiring the write lock + for (int i = 0; i < readHoldCount; i++) { + rwl.readLock().unlock(); + } } - } else { - readWriteLock.writeLock().lock(); - locked = true; + if (time > 0) { + locked = rwl.writeLock().tryLock(time, lock.unit()); + if (!locked) { + throw new LockException("Write lock not acquired in " + lock.unit().toMillis(time) + " ms"); + } + } else { + rwl.writeLock().lock(); + locked = true; + } + } finally { + rl.unlock(); } return ctx.proceed(); } finally { if (locked) { - readWriteLock.writeLock().unlock(); + if (readHoldCount > 0) { + // Re-aqcquire the read locks + for (int i = 0; i < readHoldCount; i++) { + rwl.readLock().lock(); + } + } + rwl.writeLock().unlock(); } } } @@ -63,32 +84,29 @@ private Object readLock(Lock lock, InvocationContext ctx) throws Exception { long time = lock.time(); try { if (time > 0) { - locked = readWriteLock.readLock().tryLock(time, lock.unit()); + locked = rwl.readLock().tryLock(time, lock.unit()); if (!locked) { throw new LockException("Read lock not acquired in " + lock.unit().toMillis(time) + " ms"); } } else { - readWriteLock.readLock().lock(); + rwl.readLock().lock(); locked = true; } return ctx.proceed(); } finally { if (locked) { - readWriteLock.readLock().unlock(); + rwl.readLock().unlock(); } } } - @SuppressWarnings("unchecked") - Lock getLock(InvocationContext ctx) { - Set bindings = (Set) ctx.getContextData().get(ArcInvocationContext.KEY_INTERCEPTOR_BINDINGS); - for (Annotation annotation : bindings) { - if (annotation.annotationType().equals(Lock.class)) { - return (Lock) annotation; - } + Lock getLock(ArcInvocationContext ctx) { + Lock lock = ctx.findIterceptorBinding(Lock.class); + if (lock == null) { + // This should never happen + throw new LockException("@Lock binding not found on business method " + ctx.getMethod()); } - // This should never happen - throw new LockException("@Lock binding not found on business method " + ctx.getMethod()); + return lock; } } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lock/LockInterceptorDeadlockTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lock/LockInterceptorDeadlockTest.java new file mode 100644 index 0000000000000..809d8a57df5bd --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/lock/LockInterceptorDeadlockTest.java @@ -0,0 +1,53 @@ +package io.quarkus.arc.test.lock; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.enterprise.context.ApplicationScoped; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.Lock; +import io.quarkus.arc.Lock.Type; +import io.quarkus.arc.impl.LockInterceptor; +import io.quarkus.arc.test.ArcTestContainer; + +public class LockInterceptorDeadlockTest { + + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(SimpleApplicationScopedBean.class, Lock.class, + LockInterceptor.class); + + @Test + public void testApplicationScopedBean() throws Exception { + SimpleApplicationScopedBean bean = Arc.container().instance(SimpleApplicationScopedBean.class).get(); + assertTrue(bean.read()); + assertTrue(bean.nestedRead()); + assertTrue(bean.nestedWrite()); + } + + @ApplicationScoped + static class SimpleApplicationScopedBean { + + @Lock(Type.READ) + boolean read() { + return write(); + } + + @Lock(Type.READ) + boolean nestedRead() { + return read(); + } + + @Lock(Type.WRITE) + boolean write() { + return true; + } + + @Lock(Type.WRITE) + boolean nestedWrite() { + return nestedRead(); + } + } +}