fix: Data race on failedPromises in DeterministicRunnerImpl

[hcantunc]

What changed?
Replace HashSet with ConcurrentHashMap.newKeySet() for failedPromises in DeterministicRunnerImpl.

Why?
failedPromises is written to via registerFailedPromise/forgetFailedPromise without holding the lock, while close() reads it under the lock on another thread.

How did you test it?
Existing unit tests.

Potential risks
Minimal. ConcurrentHashMap.newKeySet() is a drop-in replacement for HashSet but has slightly higher memory overhead and per-operation.

Release notes
N/A

Documentation Changes
N/A

[gitar-bot]

CI failed: A unit test failure in 'WorkflowTest.testChildWorkflowWithCronSchedule' indicates a regression, where a cron workflow executed four times instead of the expected two. This likely stems from the recent changes to 'DeterministicRunnerImpl' intended to fix a data race.

Overview

One unit test failure was identified in the WorkflowTest.testChildWorkflowWithCronSchedule suite while running in 'Sticky ON' mode. The failure suggests a logic regression in how cron-scheduled workflows are managed or re-executed following the PR's concurrency changes.

Failures

WorkflowTest.testChildWorkflowWithCronSchedule Failure (confidence: medium)

• Type: test
• Affected jobs: 79169824720
• Related to change: yes
• Root cause: The test reported an org.junit.ComparisonFailure (expected 'run 2' but was 'run 4'). This suggests that the synchronization changes in DeterministicRunnerImpl may have inadvertently caused multiple executions or an improper promise resolution for child workflows using cron schedules.
• Suggested fix: Review the failedPromises data race fix in DeterministicRunnerImpl.java. Specifically, check if the synchronization logic allows premature re-execution or if it alters the state of the promise chain in a way that triggers duplicate cron activations. Ensure that the fix does not interfere with the expected execution count logic.

Summary

• Change-related failures: 1 (Unit test regression in WorkflowTest related to cron workflow scheduling).
• Infrastructure/flaky failures: 0
• Recommended action: Verify the logic in DeterministicRunnerImpl.java against the Cron Workflow execution lifecycle. The author should investigate if the synchronization scope is too broad or if it impacts the re-entrancy of the testChildWorkflowWithCronSchedule test case.

Code Review ✅ Approved

Replaces HashSet with ConcurrentHashMap.newKeySet() in DeterministicRunnerImpl to eliminate the data race between lockless writes and locked reads. No issues found.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[bowenxia]

Do we have a unit test to cover the Data race on failedPromises case?