Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArC: fix deadlock when calling guarded methods on the same thread #35140

Merged
merged 1 commit into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
}
}
Loading