Skip to content

Commit dcebeed

Browse files
authored
Fix Fray hang when a thread exits but the monitor lock is acquired by another thread. (#151)
1 parent 0f5cb37 commit dcebeed

File tree

5 files changed

+36
-22
lines changed

5 files changed

+36
-22
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
### Fixed
1414

15+
- Fix Fray hang when a thread exits but the monitor lock is not released.
16+
1517
### Security
1618

1719
## 0.3.0 - 2025-03-10

core/src/main/kotlin/org/pastalab/fray/core/RunContext.kt

+15-19
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ class RunContext(val config: Configuration) {
337337
fun threadCompleted(t: Thread) {
338338
val context = registeredThreads[t.id]!!
339339
context.isExiting = true
340-
monitorEnter(t)
340+
monitorEnter(t, true)
341341
objectNotifyAll(t)
342342
context.state = ThreadState.Completed
343343
// lockManager.threadUnblockedDueToDeadlock(t)
@@ -539,15 +539,16 @@ class RunContext(val config: Configuration) {
539539
}
540540

541541
fun lockTryLock(lock: Any, canInterrupt: Boolean, timed: Boolean) {
542-
lockImpl(lock, false, false, canInterrupt, timed)
542+
lockImpl(lock, false, false, canInterrupt, timed, false)
543543
}
544544

545545
fun lockImpl(
546546
lock: Any,
547547
isMonitorLock: Boolean,
548548
shouldBlock: Boolean,
549549
canInterrupt: Boolean,
550-
timed: Boolean
550+
timed: Boolean,
551+
shouldRetry: Boolean,
551552
) {
552553
val t = Thread.currentThread().id
553554
val context = registeredThreads[t]!!
@@ -587,7 +588,11 @@ class RunContext(val config: Configuration) {
587588
// not want to rely on ReentrantLock. This allows
588589
// us to pick which Thread to run next if multiple
589590
// threads hold the same lock.
590-
scheduleNextOperation(true)
591+
if (shouldRetry) {
592+
scheduleNextOperationAndCheckDeadlock(true)
593+
} else {
594+
scheduleNextOperation(true)
595+
}
591596
if (canInterrupt) {
592597
context.checkInterrupt()
593598
}
@@ -599,12 +604,12 @@ class RunContext(val config: Configuration) {
599604
}
600605
}
601606

602-
fun monitorEnter(lock: Any) {
603-
lockImpl(lock, true, true, false, false)
607+
fun monitorEnter(lock: Any, shouldRetry: Boolean) {
608+
lockImpl(lock, true, true, false, false, shouldRetry)
604609
}
605610

606611
fun lockLock(lock: Any, canInterrupt: Boolean) {
607-
lockImpl(lock, false, true, canInterrupt, false)
612+
lockImpl(lock, false, true, canInterrupt, false, false)
608613
}
609614

610615
fun reentrantReadWriteLockInit(
@@ -1028,18 +1033,9 @@ class RunContext(val config: Configuration) {
10281033

10291034
// The second empty check throws deadlock exceptions.
10301035
if (enabledOperations.isEmpty()) {
1031-
if (registeredThreads.all { it.value.state == ThreadState.Completed }) {
1032-
// We are done here, we should go back to the main thread.
1033-
if (currentThreadId != mainThreadId) {
1034-
registeredThreads[mainThreadId]!!.unblock()
1035-
}
1036-
return
1037-
} else if (!currentThread.isExiting || Thread.currentThread() is HelperThread) {
1038-
// Deadlock detected
1039-
val e = org.pastalab.fray.runtime.DeadlockException()
1040-
reportError(e)
1041-
throw e
1042-
}
1036+
val e = DeadlockException()
1037+
reportError(e)
1038+
throw e
10431039
}
10441040

10451041
step += 1

core/src/main/kotlin/org/pastalab/fray/core/RuntimeDelegate.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ class RuntimeDelegate(val context: RunContext) : org.pastalab.fray.runtime.Deleg
239239
override fun onMonitorEnter(o: Any) {
240240
if (checkEntered()) return
241241
try {
242-
context.monitorEnter(o)
242+
context.monitorEnter(o, false)
243243
} finally {
244244
entered.set(false)
245245
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package org.pastalab.fray.test.fail.thread;
2+
3+
import java.util.concurrent.CountDownLatch;
4+
5+
public class ThreadExitDeadlock {
6+
public static void main(String[] args) throws InterruptedException {
7+
CountDownLatch latch = new CountDownLatch(1);
8+
Thread t = new Thread(() -> {
9+
});
10+
synchronized (t) {
11+
t.start();
12+
latch.await();
13+
}
14+
}
15+
}

integration-test/src/test/java/org/pastalab/fray/test/FrayTestCase.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.pastalab.fray.test.fail.cdl.CountDownLatchDeadlockUnblockMultiThread;
1818
import org.pastalab.fray.test.fail.park.ParkDeadlock;
1919
import org.pastalab.fray.test.fail.rwlock.ReentrantReadWriteLockDeadlock;
20+
import org.pastalab.fray.test.fail.thread.ThreadExitDeadlock;
2021
import org.pastalab.fray.test.fail.wait.NotifyOrder;
2122
import org.pastalab.fray.test.success.condition.ConditionAwaitTimeoutNotifyInterrupt;
2223
import org.pastalab.fray.test.success.rwlock.ReentrantReadWriteLockDowngradingNoDeadlock;
@@ -77,7 +78,7 @@ public void testOne() throws Throwable {
7778
new ExecutionInfo(
7879
new LambdaExecutor(() -> {
7980
try {
80-
NotifyOrder.main(new String[]{});
81+
ThreadExitDeadlock.main(new String[]{});
8182
} catch (Exception e) {
8283
throw new RuntimeException(e);
8384
}
@@ -90,7 +91,7 @@ public void testOne() throws Throwable {
9091
"/tmp/report2",
9192
1000,
9293
60,
93-
new RandomScheduler(),
94+
new POSScheduler(),
9495
new ControlledRandom(),
9596
true,
9697
false,

0 commit comments

Comments
 (0)