Skip to content

Commit 08096cf

Browse files
tishunRoiocam
andauthored
fix:deadlock when reentrant exclusive lock #2905 #2879 (#2961) (#3041)
* fix:deadlock when reentrant exclusive lock #2905 * confirm won't blocking other thread * apply suggestions Co-authored-by: Andy(Jingzhang)Chen <[email protected]>
1 parent 4280728 commit 08096cf

File tree

2 files changed

+68
-2
lines changed

2 files changed

+68
-2
lines changed

src/main/java/io/lettuce/core/protocol/SharedLock.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class SharedLock {
2626

2727
private final Lock lock = new ReentrantLock();
2828

29+
private final ThreadLocal<Integer> threadWriters = ThreadLocal.withInitial(() -> 0);
30+
2931
private volatile long writers = 0;
3032

3133
private volatile Thread exclusiveLockOwner;
@@ -45,6 +47,7 @@ void incrementWriters() {
4547

4648
if (WRITERS.get(this) >= 0) {
4749
WRITERS.incrementAndGet(this);
50+
threadWriters.set(threadWriters.get() + 1);
4851
return;
4952
}
5053
}
@@ -63,6 +66,7 @@ void decrementWriters() {
6366
}
6467

6568
WRITERS.decrementAndGet(this);
69+
threadWriters.set(threadWriters.get() - 1);
6670
}
6771

6872
/**
@@ -121,7 +125,8 @@ private void lockWritersExclusive() {
121125
try {
122126
for (;;) {
123127

124-
if (WRITERS.compareAndSet(this, 0, -1)) {
128+
// allow reentrant exclusive lock by comparing writers count and threadWriters count
129+
if (WRITERS.compareAndSet(this, threadWriters.get(), -1)) {
125130
exclusiveLockOwner = Thread.currentThread();
126131
return;
127132
}
@@ -137,9 +142,13 @@ private void lockWritersExclusive() {
137142
private void unlockWritersExclusive() {
138143

139144
if (exclusiveLockOwner == Thread.currentThread()) {
140-
if (WRITERS.incrementAndGet(this) == 0) {
145+
// check exclusive look not reentrant first
146+
if (WRITERS.compareAndSet(this, -1, threadWriters.get())) {
141147
exclusiveLockOwner = null;
148+
return;
142149
}
150+
// otherwise unlock until no more reentrant left
151+
WRITERS.incrementAndGet(this);
143152
}
144153
}
145154

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package io.lettuce.core.protocol;
2+
3+
import org.junit.jupiter.api.Assertions;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.util.concurrent.CountDownLatch;
7+
import java.util.concurrent.TimeUnit;
8+
9+
public class SharedLockTest {
10+
11+
@Test
12+
public void safety_on_reentrant_lock_exclusive_on_writers() throws InterruptedException {
13+
final SharedLock sharedLock = new SharedLock();
14+
CountDownLatch cnt = new CountDownLatch(1);
15+
try {
16+
sharedLock.incrementWriters();
17+
18+
String result = sharedLock.doExclusive(() -> {
19+
return sharedLock.doExclusive(() -> {
20+
return "ok";
21+
});
22+
});
23+
if ("ok".equals(result)) {
24+
cnt.countDown();
25+
}
26+
} finally {
27+
sharedLock.decrementWriters();
28+
}
29+
30+
boolean await = cnt.await(1, TimeUnit.SECONDS);
31+
Assertions.assertTrue(await);
32+
33+
// verify writers won't be negative after finally decrementWriters
34+
String result = sharedLock.doExclusive(() -> {
35+
return sharedLock.doExclusive(() -> {
36+
return "ok";
37+
});
38+
});
39+
40+
Assertions.assertEquals("ok", result);
41+
42+
// and other writers should be passed after exclusive lock released
43+
CountDownLatch cntOtherThread = new CountDownLatch(1);
44+
new Thread(() -> {
45+
try {
46+
sharedLock.incrementWriters();
47+
cntOtherThread.countDown();
48+
} finally {
49+
sharedLock.decrementWriters();
50+
}
51+
}).start();
52+
53+
await = cntOtherThread.await(1, TimeUnit.SECONDS);
54+
Assertions.assertTrue(await);
55+
}
56+
57+
}

0 commit comments

Comments
 (0)