From 2fd410eef77095a34e1a60deb5275aab73f5e72d Mon Sep 17 00:00:00 2001 From: Samuel Vila Date: Sun, 11 Oct 2026 20:43:09 +0200 Subject: [PATCH] Release time skipping lock when parent is closed When a child workflow closes, the test server locks time skipping until the parent processes the close notification. The parent released that lock while handling the notification, but if the parent had already closed, as is usual for an abandoned child, its update failed with NOT_FOUND before the unlock was applied. The lock leaked and time skipping stayed disabled for the lifetime of the test server, so every later sleep, timer and retry backoff ran in real time. The child now releases the lock itself after notifying the parent, whether or not the notification succeeded. This keeps the lock and unlock together and guarantees a single release. By the time the parent's update returns it holds its own workflow task lock, so time still cannot skip before the parent reacts. Fixes #2392. --- CHANGELOG.md | 2 + .../TestWorkflowMutableStateImpl.java | 28 +++- .../AbandonedChildTimeSkippingTest.java | 146 ++++++++++++++++++ 3 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 temporal-test-server/src/test/java/io/temporal/testserver/functional/timeskipping/AbandonedChildTimeSkippingTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index af4bac6c19..b4390df75b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ to docs, or any other relevant information. ### Fixed - Test server now honors retry expiration deadlines that fall exactly on a whole second. Previously such deadlines were ignored and retries were scheduled past them instead of failing with `RETRY_STATE_TIMEOUT`. +- Test server no longer disables time skipping when a child workflow closes after its parent, for example an abandoned + child. Previously every later sleep or timer on that test server ran in real time. ## Previous releases diff --git a/temporal-test-server/src/main/java/io/temporal/internal/testservice/TestWorkflowMutableStateImpl.java b/temporal-test-server/src/main/java/io/temporal/internal/testservice/TestWorkflowMutableStateImpl.java index 804cf7906c..a96bf62fc4 100644 --- a/temporal-test-server/src/main/java/io/temporal/internal/testservice/TestWorkflowMutableStateImpl.java +++ b/temporal-test-server/src/main/java/io/temporal/internal/testservice/TestWorkflowMutableStateImpl.java @@ -1385,7 +1385,6 @@ public void childWorkflowFailed( child.action(StateMachines.Action.FAIL, ctx, a, 0); childWorkflows.remove(a.getInitiatedEventId()); scheduleWorkflowTask(ctx); - ctx.unlockTimer("childWorkflowFailed"); }); } @@ -1398,7 +1397,6 @@ public void childWorkflowTimedOut( child.action(Action.TIME_OUT, ctx, a.getRetryState(), 0); childWorkflows.remove(a.getInitiatedEventId()); scheduleWorkflowTask(ctx); - ctx.unlockTimer("childWorkflowTimedOut"); }); } @@ -1424,7 +1422,6 @@ public void childWorkflowCompleted( child.action(StateMachines.Action.COMPLETE, ctx, a, 0); childWorkflows.remove(a.getInitiatedEventId()); scheduleWorkflowTask(ctx); - ctx.unlockTimer("childWorkflowCompleted"); }); } @@ -1437,7 +1434,6 @@ public void childWorkflowCanceled( child.action(StateMachines.Action.CANCEL, ctx, a, 0); childWorkflows.remove(a.getInitiatedEventId()); scheduleWorkflowTask(ctx); - ctx.unlockTimer("childWorkflowCanceled"); }); } @@ -1594,7 +1590,8 @@ private void processFailWorkflowExecution( workflowTaskStateMachine.getData().workflowCompleted = true; processWorkflowCompletionCallbacks(ctx); if (parent.isPresent()) { - ctx.lockTimer("processFailWorkflowExecution notify parent"); // unlocked by the parent + // Keeps time from skipping until the parent schedules a workflow task. + ctx.lockTimer("processFailWorkflowExecution notify parent"); ChildWorkflowExecutionFailedEventAttributes a = ChildWorkflowExecutionFailedEventAttributes.newBuilder() .setInitiatedEventId(parentChildInitiatedEventId.getAsLong()) @@ -1617,6 +1614,9 @@ private void processFailWorkflowExecution( } } catch (Throwable e) { log.error("Failure reporting child failure", e); + } finally { + // Released even if the parent already closed, so time skipping is not stuck. + timerService.unlockTimeSkipping("processFailWorkflowExecution notify parent"); } }); } @@ -1641,7 +1641,8 @@ private void processCompleteWorkflowExecution( // years workflow.getData().runTimerCancellationHandle.apply(); if (parent.isPresent()) { - ctx.lockTimer("processCompleteWorkflowExecution notify parent"); // unlocked by the parent + // Keeps time from skipping until the parent schedules a workflow task. + ctx.lockTimer("processCompleteWorkflowExecution notify parent"); ChildWorkflowExecutionCompletedEventAttributes a = ChildWorkflowExecutionCompletedEventAttributes.newBuilder() .setInitiatedEventId(parentChildInitiatedEventId.getAsLong()) @@ -1665,6 +1666,9 @@ private void processCompleteWorkflowExecution( } } catch (Throwable e) { log.error("Failure reporting child completion", e); + } finally { + // Released even if the parent already closed, so time skipping is not stuck. + timerService.unlockTimeSkipping("processCompleteWorkflowExecution notify parent"); } }); } @@ -1718,7 +1722,8 @@ private void processCancelWorkflowExecution( workflowTaskStateMachine.getData().workflowCompleted = true; processWorkflowCompletionCallbacks(ctx); if (parent.isPresent()) { - ctx.lockTimer("processCancelWorkflowExecution notify parent"); // unlocked by the parent + // Keeps time from skipping until the parent schedules a workflow task. + ctx.lockTimer("processCancelWorkflowExecution notify parent"); ChildWorkflowExecutionCanceledEventAttributes a = ChildWorkflowExecutionCanceledEventAttributes.newBuilder() .setInitiatedEventId(parentChildInitiatedEventId.getAsLong()) @@ -1742,6 +1747,9 @@ private void processCancelWorkflowExecution( } } catch (Throwable e) { log.error("Failure reporting child cancellation", e); + } finally { + // Released even if the parent already closed, so time skipping is not stuck. + timerService.unlockTimeSkipping("processCancelWorkflowExecution notify parent"); } }); } @@ -2646,7 +2654,8 @@ private void timeoutWorkflow() { workflowTaskStateMachine.getData().workflowCompleted = true; processWorkflowCompletionCallbacks(ctx); if (parent.isPresent()) { - ctx.lockTimer("timeoutWorkflow notify parent"); // unlocked by the parent + // Keeps time from skipping until the parent schedules a workflow task. + ctx.lockTimer("timeoutWorkflow notify parent"); } ForkJoinPool.commonPool().execute(() -> reportWorkflowTimeoutToParent(ctx)); }); @@ -2677,6 +2686,9 @@ private void reportWorkflowTimeoutToParent(RequestContext ctx) { } } catch (Exception e) { log.error("Failure reporting child timing out", e); + } finally { + // Released even if the parent already closed, so time skipping is not stuck. + timerService.unlockTimeSkipping("timeoutWorkflow notify parent"); } } diff --git a/temporal-test-server/src/test/java/io/temporal/testserver/functional/timeskipping/AbandonedChildTimeSkippingTest.java b/temporal-test-server/src/test/java/io/temporal/testserver/functional/timeskipping/AbandonedChildTimeSkippingTest.java new file mode 100644 index 0000000000..2f17ac4711 --- /dev/null +++ b/temporal-test-server/src/test/java/io/temporal/testserver/functional/timeskipping/AbandonedChildTimeSkippingTest.java @@ -0,0 +1,146 @@ +package io.temporal.testserver.functional.timeskipping; + +import static org.junit.Assume.assumeFalse; + +import io.temporal.api.enums.v1.ParentClosePolicy; +import io.temporal.client.WorkflowFailedException; +import io.temporal.client.WorkflowOptions; +import io.temporal.client.WorkflowStub; +import io.temporal.failure.ApplicationFailure; +import io.temporal.testing.internal.SDKTestWorkflowRule; +import io.temporal.workflow.Async; +import io.temporal.workflow.ChildWorkflowOptions; +import io.temporal.workflow.Workflow; +import io.temporal.workflow.WorkflowInterface; +import io.temporal.workflow.WorkflowMethod; +import java.time.Duration; +import java.util.UUID; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +/** + * An abandoned child that closes after its parent must not disable time skipping. The test server + * locks time skipping when a child closes until the parent processes the notification, so the lock + * has to be released when the parent is already closed. If it leaks, the final sleep runs in real + * time and the test times out. + */ +public class AbandonedChildTimeSkippingTest { + private static final Duration CHILD_SLEEP = Duration.ofHours(1); + + @Rule + public SDKTestWorkflowRule testWorkflowRule = + SDKTestWorkflowRule.newBuilder() + .setWorkflowTypes(ParentWorkflowImpl.class, ChildWorkflowImpl.class) + .build(); + + @Before + public void checkRealServer() { + assumeFalse("Time skipping is a test server feature", SDKTestWorkflowRule.useExternalService); + } + + @Test + public void timeSkippingWorksAfterAbandonedChildCompletes() { + String childId = startParentWithAbandonedChild(ChildOutcome.COMPLETE); + WorkflowStub child = testWorkflowRule.getWorkflowClient().newUntypedWorkflowStub(childId); + child.getResult(Void.class); + assertTimeSkippingWorks(); + } + + @Test + public void timeSkippingWorksAfterAbandonedChildFails() { + String childId = startParentWithAbandonedChild(ChildOutcome.FAIL); + WorkflowStub child = testWorkflowRule.getWorkflowClient().newUntypedWorkflowStub(childId); + awaitFailure(child); + assertTimeSkippingWorks(); + } + + @Test + public void timeSkippingWorksAfterAbandonedChildIsCanceled() { + String childId = startParentWithAbandonedChild(ChildOutcome.CANCEL); + WorkflowStub child = testWorkflowRule.getWorkflowClient().newUntypedWorkflowStub(childId); + child.cancel(); + awaitFailure(child); + assertTimeSkippingWorks(); + } + + @Test + public void timeSkippingWorksAfterAbandonedChildTimesOut() { + String childId = startParentWithAbandonedChild(ChildOutcome.TIME_OUT); + WorkflowStub child = testWorkflowRule.getWorkflowClient().newUntypedWorkflowStub(childId); + awaitFailure(child); + assertTimeSkippingWorks(); + } + + private String startParentWithAbandonedChild(ChildOutcome outcome) { + String childId = UUID.randomUUID().toString(); + ParentWorkflow parent = + testWorkflowRule + .getWorkflowClient() + .newWorkflowStub( + ParentWorkflow.class, + WorkflowOptions.newBuilder().setTaskQueue(testWorkflowRule.getTaskQueue()).build()); + // The parent returns as soon as the child has started. + parent.execute(childId, outcome); + return childId; + } + + private static void awaitFailure(WorkflowStub child) { + try { + child.getResult(Void.class); + } catch (WorkflowFailedException expected) { + return; + } + throw new AssertionError("Child workflow was expected to fail"); + } + + private void assertTimeSkippingWorks() { + // Blocks in real time, exceeding the test timeout, if time skipping is locked. + testWorkflowRule.getTestEnvironment().sleep(Duration.ofDays(1)); + } + + public enum ChildOutcome { + COMPLETE, + FAIL, + CANCEL, + TIME_OUT + } + + @WorkflowInterface + public interface ParentWorkflow { + @WorkflowMethod + void execute(String childId, ChildOutcome outcome); + } + + @WorkflowInterface + public interface ChildWorkflow { + @WorkflowMethod + void execute(ChildOutcome outcome); + } + + public static class ParentWorkflowImpl implements ParentWorkflow { + @Override + public void execute(String childId, ChildOutcome outcome) { + ChildWorkflowOptions.Builder options = + ChildWorkflowOptions.newBuilder() + .setWorkflowId(childId) + .setParentClosePolicy(ParentClosePolicy.PARENT_CLOSE_POLICY_ABANDON); + if (outcome == ChildOutcome.TIME_OUT) { + options.setWorkflowRunTimeout(CHILD_SLEEP.dividedBy(2)); + } + ChildWorkflow child = Workflow.newChildWorkflowStub(ChildWorkflow.class, options.build()); + Async.procedure(child::execute, outcome); + Workflow.getWorkflowExecution(child).get(); + } + } + + public static class ChildWorkflowImpl implements ChildWorkflow { + @Override + public void execute(ChildOutcome outcome) { + Workflow.sleep(CHILD_SLEEP); + if (outcome == ChildOutcome.FAIL) { + throw ApplicationFailure.newFailure("Child failed", "ChildFailure"); + } + } + } +}