Skip to content

Commit

Permalink
ArC: fix deadlock when calling guarded methods on the same thread
Browse files Browse the repository at this point in the history
  • Loading branch information
mkouba committed Aug 1, 2023
1 parent 773b8c1 commit e6c044e
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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();
}
}
}
Expand All @@ -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<Annotation> bindings = (Set<Annotation>) 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;
}

}
Original file line number Diff line number Diff line change
@@ -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();
}
}
}

0 comments on commit e6c044e

Please sign in to comment.