Skip to content

Commit fda4d0a

Browse files
Fix race in AsyncPollerTest.testSuspendPolling
The test relied on the poller's second iteration racing past its suspend-latch check before the test thread called suspendPolling(). If the test thread won the race, iter 2 blocked at the suspend latch and never reached reserveSlot — so reservedCount stayed at 1 and the later `expected:<2> but was:<1>` assertion failed. Bumping the assertEventually timeout didn't help; the race either wins or loses deterministically on a given run. Remove the race: override reserveSlot on the slot supplier to count invocations, and wait for reserveCalls == 2 before suspending. That's a direct signal that iter 2 has passed the suspend check and called reserveSlot (its future can't complete until iter 1's slot is released, but the call itself happens immediately). After this signal, iter 2 is pinned past the suspend check, so the subsequent suspendPolling() only affects iter 3+. Also drop the now-redundant first assertEventually: once pollLatch counts down (inside the mocked poll()), iter 1's slot has already been issued, so reservedCount == 1 is synchronously visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b6f4283 commit fda4d0a

1 file changed

Lines changed: 21 additions & 10 deletions

File tree

temporal-sdk/src/test/java/io/temporal/internal/worker/AsyncPollerTest.java

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,15 @@ public void testAsyncPollFailed()
336336
@Test
337337
public void testSuspendPolling()
338338
throws InterruptedException, ExecutionException, AsyncPoller.PollTaskAsyncAbort {
339-
CountingSlotSupplier<SlotInfo> slotSupplierInner = new CountingSlotSupplier<>(1);
339+
AtomicInteger reserveCalls = new AtomicInteger();
340+
CountingSlotSupplier<SlotInfo> slotSupplierInner =
341+
new CountingSlotSupplier<SlotInfo>(1) {
342+
@Override
343+
public SlotSupplierFuture reserveSlot(SlotReserveContext<SlotInfo> ctx) throws Exception {
344+
reserveCalls.incrementAndGet();
345+
return super.reserveSlot(ctx);
346+
}
347+
};
340348
TrackingSlotSupplier<?> slotSupplier =
341349
new TrackingSlotSupplier<>(slotSupplierInner, new NoopScope());
342350
DummyTaskExecutor executor = new DummyTaskExecutor(slotSupplier);
@@ -370,18 +378,21 @@ public void testSuspendPolling()
370378
poller.resumePolling();
371379
assertFalse(poller.isSuspended());
372380
pollLatch.await();
373-
assertEventually(
374-
Duration.ofSeconds(1),
375-
() -> {
376-
assertEquals(0, executor.processed.get());
377-
assertEquals(1, slotSupplierInner.reservedCount.get());
378-
assertEquals(0, slotSupplier.getUsedSlots().size());
379-
});
380-
// Suspend polling again, this will not affect the already issued poll request
381+
assertEquals(0, executor.processed.get());
382+
assertEquals(1, slotSupplierInner.reservedCount.get());
383+
assertEquals(0, slotSupplier.getUsedSlots().size());
384+
// Wait for iter 2 of the poll loop to invoke reserveSlot. Completion of that reservation
385+
// requires iter 1's slot to be released (via completePoll below) since the supplier has
386+
// only one slot — but the *call* to reserveSlot happens as soon as iter 2 passes the
387+
// suspend-latch check. Waiting on that call here removes a race where the suspendPolling
388+
// below could otherwise strand iter 2 at the suspend latch before it reserves.
389+
assertEventually(Duration.ofSeconds(5), () -> assertEquals(2, reserveCalls.get()));
390+
// Suspend polling again, this will not affect the already issued poll request or the
391+
// iter-2 reserveSlot call above (which is now waiting for the slot to free up).
381392
poller.suspendPolling();
382393
completePoll.get().apply();
383394
assertEventually(
384-
Duration.ofSeconds(1),
395+
Duration.ofSeconds(5),
385396
() -> {
386397
assertEquals(1, executor.processed.get());
387398
assertEquals(2, slotSupplierInner.reservedCount.get());

0 commit comments

Comments
 (0)