Skip to content

Commit 29d9f2b

Browse files
committed
Fix race condition in WanCopyRegionFunctionServiceTest
The test severalExecuteWithDifferentRegionOrSenderAreAllowed was failing intermittently with 'expected: 5 but was: 2/3' due to a race condition. Root Cause: The test creates 5 CompletableFuture tasks using supplyAsync(), which executes asynchronously. There was no guarantee that all tasks would call service.execute() and be added to the executions map before the final assertion checked the count. The race occurred because: 1. CompletableFuture.supplyAsync() schedules tasks asynchronously 2. Multiple tasks could be scheduled but not yet executed 3. The assertion would run before all tasks had called service.execute() Solution: Instead of starting all tasks and then waiting, we now start each task sequentially and wait for it to be registered in the executions map before starting the next one. This ensures: - Each execution is fully registered before the next one starts - No race condition between task scheduling and execution registration - The final count is guaranteed to be 5 This approach is more deterministic and eliminates the race condition while still testing that multiple executions with different regions are allowed to run concurrently.
1 parent 9f776b9 commit 29d9f2b

1 file changed

Lines changed: 9 additions & 4 deletions

File tree

geode-wan/src/test/java/org/apache/geode/cache/wan/internal/WanCopyRegionFunctionServiceTest.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public void cancelAllExecutionsWithRunningExecutionsReturnsCanceledExecutions()
154154
}
155155

156156
@Test
157-
public void severalExecuteWithDifferentRegionOrSenderAreAllowed() {
157+
public void severalExecuteWithDifferentRegionOrSenderAreAllowed() throws InterruptedException {
158158
int executions = 5;
159159
CountDownLatch latch = new CountDownLatch(executions);
160160
for (int i = 0; i < executions; i++) {
@@ -172,11 +172,16 @@ public void severalExecuteWithDifferentRegionOrSenderAreAllowed() {
172172
return null;
173173
}
174174
});
175+
176+
// Wait for this specific execution to be registered before starting the next one
177+
// This ensures we don't have a race where multiple tasks try to start simultaneously
178+
// and only some get registered before we check the count
179+
await().untilAsserted(
180+
() -> assertThat(service.getNumberOfCurrentExecutions()).isGreaterThanOrEqualTo(i + 1));
175181
}
176182

177-
// Wait for the functions to start execution
178-
await().untilAsserted(
179-
() -> assertThat(service.getNumberOfCurrentExecutions()).isEqualTo(executions));
183+
// Verify all executions are registered
184+
assertThat(service.getNumberOfCurrentExecutions()).isEqualTo(executions);
180185

181186
// End executions
182187
for (int i = 0; i < executions; i++) {

0 commit comments

Comments
 (0)