Skip to content

馃挜 Preserve non-cancellable timed promise waits during cancellation - #3029

Open
Quinn-With-Two-Ns wants to merge 2 commits into
temporalio:mainfrom
Quinn-With-Two-Ns:issue-3026
Open

Quinn-With-Two-Ns wants to merge 2 commits into
temporalio:mainfrom
Quinn-With-Two-Ns:issue-3026

Conversation

@Quinn-With-Two-Ns

@Quinn-With-Two-Ns Quinn-With-Two-Ns commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What was changed

This change adds the DETACH_NON_CANCELLABLE_PROMISE_GET_TIMER SDK flag and updates timed, non-cancellable Promise.get(timeout, unit) calls to run their timeout timer in a detached cancellation scope. The timer is canceled after the promise completes. Regression and replay tests cover promise completion, timeouts, activity cancellation, child workflow cancellation, and combinations with CANCEL_AWAIT_TIMER_ON_CONDITION. The activity tests use per-test latches for deterministic synchronization.

Why?

Promise.get(timeout, unit) is non-cancellable, but cancellation of its enclosing scope previously canceled its timeout timer. This ended the wait early and incorrectly surfaced a TimeoutException, potentially hiding the actual activity or child workflow cancellation. The new SDK flag preserves replay compatibility for existing workflow histories while allowing new workflows to use the corrected behaviour.

馃挜 Breaking changes

This change can affect workflow behaviour because timed, non-cancellable Promise.get(timeout, unit) calls will no longer return early with a TimeoutException when their enclosing cancellation scope is canceled. Instead, they will continue waiting until the promise completes, fails, or reaches its actual timeout. Workflows that currently catch the premature TimeoutException and use it to select a control flow path may therefore produce different results. The behaviour is protected by an SDK flag so existing workflow histories retain their original behaviour during replay, while new workflows can adopt the corrected semantics without causing nondeterminism.

Checklist

  1. Closes Promise.get(timeout, unit) throws a misleading TimeoutException when the workflow is canceled聽#3026

  2. How was this tested:

  1. Any docs updates needed?

Note

High Risk
Changes core workflow cancellation and timed-wait semantics on replay-sensitive promise paths; workflows that relied on premature timeouts after scope cancel may behave differently once the flag is enabled.

Overview
Fixes incorrect TimeoutException from non-cancellable Promise.get(timeout, unit) when a parent CancellationScope is canceled: the scope was canceling the internal await timer and ending the wait early instead of letting the promise finish or fail.

Adds SDK flag DETACH_NON_CANCELLABLE_PROMISE_GET_TIMER (pending enable). When it is on in history, CompletablePromiseImpl runs the timed await inside a detached cancellation scope and cancels that scope when the wait ends. WorkflowInternal.checkSdkFlag exposes flag checks to sync code. Legacy behavior is unchanged when the flag is off.

Regression coverage includes parameterized integration tests (with CANCEL_AWAIT_TIMER_ON_CONDITION combinations) and 12 replay history fixtures for promise, activity, child, and completion scenarios.

Reviewed by Cursor Bugbot for commit b0b0a89. Bugbot is set up for automated code reviews on this repo. Configure here.

@Quinn-With-Two-Ns
Quinn-With-Two-Ns marked this pull request as ready for review August 27, 2026 21:00
@Quinn-With-Two-Ns
Quinn-With-Two-Ns requested a review from a team as a code owner August 27, 2026 21:00

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put resource files in a subdirectory?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put them in the folder all the existing json histories are

Comment thread temporal-sdk/src/main/java/io/temporal/internal/common/SdkFlag.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promise.get(timeout, unit) throws a misleading TimeoutException when the workflow is canceled

2 participants