Skip to content

Commit 91a92e8

Browse files
committed
Add tests to reproduce virtual thread starvation
1 parent 4804646 commit 91a92e8

6 files changed

Lines changed: 122 additions & 12 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ jobs:
147147
USER: unittest
148148
TEMPORAL_SERVICE_ADDRESS: localhost:7233
149149
USE_EXTERNAL_SERVICE: true
150-
run: ./gradlew --no-daemon :temporal-sdk:virtualThreadTests -x spotlessCheck -x spotlessApply -x spotlessJava -PtestJavaVersion=21
150+
run: ./gradlew --no-daemon :temporal-sdk:virtualThreadTests :temporal-sdk:virtualThreadStarvationTests -x spotlessCheck -x spotlessApply -x spotlessJava -PtestJavaVersion=21
151151

152152
- name: Publish Test Report
153153
uses: mikepenz/action-junit-report@bccf2e31636835cf0874589931c4116687171386 # v6

temporal-sdk/build.gradle

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,23 @@ testing {
256256
}
257257
}
258258

259+
virtualThreadStarvationTests(JvmTestSuite) {
260+
targets {
261+
all {
262+
testTask.configure {
263+
jvmArgs '-Djdk.virtualThreadScheduler.parallelism=1',
264+
'-Djdk.virtualThreadScheduler.maxPoolSize=1'
265+
if (project.hasProperty("testJavaVersion")) {
266+
javaLauncher = javaToolchains.launcherFor {
267+
languageVersion = JavaLanguageVersion.of(project.property("testJavaVersion") as int)
268+
}
269+
}
270+
shouldRunAfter(virtualThreadTests)
271+
}
272+
}
273+
}
274+
}
275+
259276
// Run the same test as the normal test task with virtual threads
260277
testsWithVirtualThreads(JvmTestSuite) {
261278
// Use the same source and resources as the main test set
@@ -287,4 +304,5 @@ testing {
287304
tasks.named('check') {
288305
dependsOn(testing.suites.jackson3Tests)
289306
dependsOn(testing.suites.virtualThreadTests)
290-
}
307+
dependsOn(testing.suites.virtualThreadStarvationTests)
308+
}

temporal-sdk/src/main/java/io/temporal/internal/sync/PotentialDeadlockException.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@ public class PotentialDeadlockException extends RuntimeException {
4343
this.detectionTimestamp = detectionTimestamp;
4444
}
4545

46+
/**
47+
* @param workflowThreadContext context of the thread that is in a potential deadlock state
48+
* @param detectionTimestamp a timestamp the deadlock was detected
49+
* @param timeoutMillis configured deadlock detection timeout in milliseconds
50+
*/
51+
PotentialDeadlockException(
52+
WorkflowThreadContext workflowThreadContext,
53+
long detectionTimestamp,
54+
long timeoutMillis) {
55+
super(
56+
"[TMPRL1101] Potential deadlock detected. Workflow thread could not start executing within "
57+
+ formatTimeout(timeoutMillis)
58+
+ ".");
59+
this.workflowThreadContext = workflowThreadContext;
60+
this.detectionTimestamp = detectionTimestamp;
61+
}
62+
4663
private static String formatTimeout(long timeoutMillis) {
4764
if (timeoutMillis % 1000 == 0) {
4865
return timeoutMillis / 1000 + "s";

temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowThreadContext.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@
1010
import java.util.concurrent.locks.Lock;
1111
import java.util.function.Supplier;
1212
import javax.annotation.Nullable;
13-
import org.slf4j.Logger;
14-
import org.slf4j.LoggerFactory;
1513

1614
class WorkflowThreadContext {
17-
private static final Logger log = LoggerFactory.getLogger(WorkflowThreadContext.class);
18-
1915
// Shared runner lock
2016
private final Lock runnerLock;
2117
private final WorkflowThreadScheduler scheduler;
@@ -241,13 +237,10 @@ public boolean runUntilBlocked(long deadlockDetectionTimeoutMs) {
241237
throw new PotentialDeadlockException(
242238
currentThread.getName(), this, detectionTimestamp, deadlockDetectionTimeoutMs);
243239
} else {
244-
// This should never happen.
245-
// We clear currentThread only after setting the status to DONE.
246-
// And we check for it by the status condition check after waking up on the condition
247-
// and acquiring the lock back
248-
log.warn("Illegal State: WorkflowThreadContext has no currentThread in {} state", status);
240+
// We clear currentThread only after setting the status to DONE, so this case should
241+
// only happen if the WorkflowThread is starving.
249242
throw new PotentialDeadlockException(
250-
"UnknownThread", this, detectionTimestamp, deadlockDetectionTimeoutMs);
243+
this, detectionTimestamp, deadlockDetectionTimeoutMs);
251244
}
252245
}
253246
Preconditions.checkState(

temporal-sdk/src/test/java/io/temporal/internal/sync/DeterministicRunnerTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
import java.util.Map;
3535
import java.util.Objects;
3636
import java.util.Optional;
37+
import java.util.concurrent.CountDownLatch;
3738
import java.util.concurrent.ExecutorService;
39+
import java.util.concurrent.Executors;
3840
import java.util.concurrent.SynchronousQueue;
3941
import java.util.concurrent.ThreadPoolExecutor;
4042
import java.util.concurrent.TimeUnit;
@@ -820,6 +822,37 @@ public void testRejectedExecutionError() {
820822
}
821823
}
822824

825+
@Test
826+
public void testThreadStarvationBeforeWorkflowThreadStarts() throws InterruptedException {
827+
ExecutorService executor = Executors.newSingleThreadExecutor();
828+
CountDownLatch executorThreadOccupied = new CountDownLatch(1);
829+
CountDownLatch releaseExecutorThread = new CountDownLatch(1);
830+
executor.submit(
831+
() -> {
832+
executorThreadOccupied.countDown();
833+
releaseExecutorThread.await();
834+
return null;
835+
});
836+
executorThreadOccupied.await();
837+
838+
DeterministicRunner runner =
839+
new DeterministicRunnerImpl(
840+
executor::submit,
841+
DummySyncWorkflowContext.newDummySyncWorkflowContext(),
842+
() -> fail("workflow code should not start while the executor thread is occupied"));
843+
844+
try {
845+
PotentialDeadlockException e =
846+
Assert.assertThrows(
847+
PotentialDeadlockException.class, () -> runner.runUntilAllBlocked(10));
848+
assertTrue(e.getMessage().contains("could not start executing within 10ms"));
849+
} finally {
850+
executor.shutdownNow();
851+
releaseExecutorThread.countDown();
852+
assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
853+
}
854+
}
855+
823856
@Test
824857
public void testCloseBlockedUntilDone() throws InterruptedException {
825858
final int THREAD_SLEEP_MS = 2000;
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package io.temporal.internal.sync;
2+
3+
import static org.junit.Assert.assertThrows;
4+
import static org.junit.Assert.assertTrue;
5+
6+
import java.util.concurrent.CountDownLatch;
7+
import java.util.concurrent.ExecutorService;
8+
import java.util.concurrent.Executors;
9+
import java.util.concurrent.atomic.AtomicBoolean;
10+
import org.junit.Test;
11+
12+
public class ThreadStarvationVirtualThreadTest {
13+
14+
@Test(timeout = 10_000)
15+
public void workflowThreadDoesNotStartWhenAllCarriersAreBusy() throws Exception {
16+
CountDownLatch carrierOccupied = new CountDownLatch(1);
17+
AtomicBoolean releaseCarrier = new AtomicBoolean();
18+
Thread carrierBlocker =
19+
Thread.ofVirtual()
20+
.name("carrier-blocker")
21+
.start(
22+
() -> {
23+
carrierOccupied.countDown();
24+
while (!releaseCarrier.get()) {
25+
Thread.onSpinWait();
26+
}
27+
});
28+
carrierOccupied.await();
29+
30+
ExecutorService workflowThreadExecutor = Executors.newVirtualThreadPerTaskExecutor();
31+
DeterministicRunner runner =
32+
DeterministicRunner.newRunner(
33+
workflowThreadExecutor::submit,
34+
DummySyncWorkflowContext.newDummySyncWorkflowContext(),
35+
() -> {
36+
throw new AssertionError("workflow code should not start while the carrier is busy");
37+
});
38+
39+
try {
40+
PotentialDeadlockException e =
41+
assertThrows(PotentialDeadlockException.class, () -> runner.runUntilAllBlocked(100));
42+
assertTrue(e.getMessage().contains("could not start executing within 100ms"));
43+
} finally {
44+
workflowThreadExecutor.shutdownNow();
45+
releaseCarrier.set(true);
46+
carrierBlocker.join();
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)