Skip to content
Draft
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ jobs:
USER: unittest
TEMPORAL_SERVICE_ADDRESS: localhost:7233
USE_EXTERNAL_SERVICE: true
run: ./gradlew --no-daemon :temporal-sdk:virtualThreadTests -x spotlessCheck -x spotlessApply -x spotlessJava -PtestJavaVersion=21
run: ./gradlew --no-daemon :temporal-sdk:virtualThreadTests :temporal-sdk:virtualThreadStarvationTests -x spotlessCheck -x spotlessApply -x spotlessJava -PtestJavaVersion=21

- name: Publish Test Report
uses: mikepenz/action-junit-report@bccf2e31636835cf0874589931c4116687171386 # v6
Expand Down
20 changes: 19 additions & 1 deletion temporal-sdk/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,23 @@ testing {
}
}

virtualThreadStarvationTests(JvmTestSuite) {
targets {
all {
testTask.configure {
jvmArgs '-Djdk.virtualThreadScheduler.parallelism=1',
'-Djdk.virtualThreadScheduler.maxPoolSize=1'
if (project.hasProperty("testJavaVersion")) {
javaLauncher = javaToolchains.launcherFor {
languageVersion = JavaLanguageVersion.of(project.property("testJavaVersion") as int)
}
}
shouldRunAfter(virtualThreadTests)
}
}
}
}

// Run the same test as the normal test task with virtual threads
testsWithVirtualThreads(JvmTestSuite) {
// Use the same source and resources as the main test set
Expand Down Expand Up @@ -287,4 +304,5 @@ testing {
tasks.named('check') {
dependsOn(testing.suites.jackson3Tests)
dependsOn(testing.suites.virtualThreadTests)
}
dependsOn(testing.suites.virtualThreadStarvationTests)
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ static DeterministicRunner newRunner(
* completed or blocked.
*
* @throws Throwable if one of the threads didn't handle an exception.
* @param deadlockDetectionTimeout the maximum time in milliseconds a thread can run without
* calling yield.
* @param deadlockDetectionTimeout the maximum time in milliseconds after a thread is
* scheduled until it yields or completes.
*/
void runUntilAllBlocked(long deadlockDetectionTimeout);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,23 @@ public class PotentialDeadlockException extends RuntimeException {
this.detectionTimestamp = detectionTimestamp;
}

/**
* @param workflowThreadContext context of the thread that is in a potential deadlock state
* @param detectionTimestamp a timestamp the deadlock was detected
* @param timeoutMillis configured deadlock detection timeout in milliseconds
*/
PotentialDeadlockException(
WorkflowThreadContext workflowThreadContext,
long detectionTimestamp,
long timeoutMillis) {
super(
"[TMPRL1101] Potential deadlock detected. Workflow thread could not start executing within "
+ formatTimeout(timeoutMillis)
+ ".");
this.workflowThreadContext = workflowThreadContext;
this.detectionTimestamp = detectionTimestamp;
}

private static String formatTimeout(long timeoutMillis) {
if (timeoutMillis % 1000 == 0) {
return timeoutMillis / 1000 + "s";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ static WorkflowThread newThread(Runnable runnable, boolean detached, String name
SyncWorkflowContext getWorkflowContext();

/**
* @param deadlockDetectionTimeoutMs maximum time in milliseconds the thread can run before
* calling yield.
* @param deadlockDetectionTimeoutMs the maximum time in milliseconds after a thread is
* scheduled until it yields or completes.
* @return true if coroutine made some progress.
*/
boolean runUntilBlocked(long deadlockDetectionTimeoutMs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@
import java.util.concurrent.locks.Lock;
import java.util.function.Supplier;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

class WorkflowThreadContext {
private static final Logger log = LoggerFactory.getLogger(WorkflowThreadContext.class);

// Shared runner lock
private final Lock runnerLock;
private final WorkflowThreadScheduler scheduler;
Expand Down Expand Up @@ -214,8 +210,8 @@ public String getYieldReason() {
}

/**
* @param deadlockDetectionTimeoutMs maximum time in milliseconds the thread can run before
* calling yield. Discarded if {@code TEMPORAL_DEBUG} env variable is set.
* @param deadlockDetectionTimeoutMs the maximum time in milliseconds after a thread is scheduled
* until it yields or completes. Discarded if {@code TEMPORAL_DEBUG} env variable is set.
* @return true if thread made some progress. Which is await was unblocked and some code after it
* * was executed.
*/
Expand All @@ -241,13 +237,10 @@ public boolean runUntilBlocked(long deadlockDetectionTimeoutMs) {
throw new PotentialDeadlockException(
currentThread.getName(), this, detectionTimestamp, deadlockDetectionTimeoutMs);
} else {
// This should never happen.
// We clear currentThread only after setting the status to DONE.
// And we check for it by the status condition check after waking up on the condition
// and acquiring the lock back
log.warn("Illegal State: WorkflowThreadContext has no currentThread in {} state", status);
// We clear currentThread only after setting the status to DONE, so this case should
// only happen if the WorkflowThread is starving.
throw new PotentialDeadlockException(
"UnknownThread", this, detectionTimestamp, deadlockDetectionTimeoutMs);
this, detectionTimestamp, deadlockDetectionTimeoutMs);
}
}
Preconditions.checkState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ public Builder setLocalActivityWorkerOnly(boolean localActivityWorkerOnly) {
/**
* @param defaultDeadlockDetectionTimeoutMs time period in ms that will be used to detect
* workflows deadlock. Default is 1000ms, which is chosen if set to zero.
* <p>Specifies an amount of time in milliseconds that workflow tasks are allowed to execute
* without interruption. If workflow task runs longer than specified interval without
* yielding (like calling an Activity), it will fail automatically.
* <p>Specifies a time interval in milliseconds within which a workflow task must
* yield (like calling an Activity) or complete. If a workflow task runs longer than
* the specified interval or takes too long to begin running, it will fail automatically.
* @return {@code this}
* @see io.temporal.internal.sync.PotentialDeadlockException
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.SynchronousQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -820,6 +822,37 @@ public void testRejectedExecutionError() {
}
}

@Test
public void testThreadStarvationBeforeWorkflowThreadStarts() throws InterruptedException {
ExecutorService executor = Executors.newSingleThreadExecutor();
CountDownLatch executorThreadOccupied = new CountDownLatch(1);
CountDownLatch releaseExecutorThread = new CountDownLatch(1);
executor.submit(
() -> {
executorThreadOccupied.countDown();
releaseExecutorThread.await();
return null;
});
executorThreadOccupied.await();

DeterministicRunner runner =
new DeterministicRunnerImpl(
executor::submit,
DummySyncWorkflowContext.newDummySyncWorkflowContext(),
() -> fail("workflow code should not start while the executor thread is occupied"));

try {
PotentialDeadlockException e =
Assert.assertThrows(
PotentialDeadlockException.class, () -> runner.runUntilAllBlocked(10));
assertTrue(e.getMessage().contains("could not start executing within 10ms"));
} finally {
executor.shutdownNow();
releaseExecutorThread.countDown();
assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
}
}

@Test
public void testCloseBlockedUntilDone() throws InterruptedException {
final int THREAD_SLEEP_MS = 2000;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package io.temporal.internal.sync;

import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Test;

public class ThreadStarvationVirtualThreadTest {

@Test(timeout = 10_000)
public void workflowThreadDoesNotStartWhenAllCarriersAreBusy() throws Exception {
CountDownLatch carrierOccupied = new CountDownLatch(1);
AtomicBoolean releaseCarrier = new AtomicBoolean();
Thread carrierBlocker =
Thread.ofVirtual()
.name("carrier-blocker")
.start(
() -> {
carrierOccupied.countDown();
while (!releaseCarrier.get()) {
Thread.onSpinWait();
}
});
carrierOccupied.await();

ExecutorService workflowThreadExecutor = Executors.newVirtualThreadPerTaskExecutor();
DeterministicRunner runner =
DeterministicRunner.newRunner(
workflowThreadExecutor::submit,
DummySyncWorkflowContext.newDummySyncWorkflowContext(),
() -> {
throw new AssertionError("workflow code should not start while the carrier is busy");
});

try {
PotentialDeadlockException e =
assertThrows(PotentialDeadlockException.class, () -> runner.runUntilAllBlocked(100));
assertTrue(e.getMessage().contains("could not start executing within 100ms"));
} finally {
workflowThreadExecutor.shutdownNow();
releaseCarrier.set(true);
carrierBlocker.join();
}
}
}
Loading