Skip to content

Commit 0d4efb1

Browse files
committed
Fix race condition in DirectRunner executor shutdown
Previously, in ExecutorServiceParallelExecutor, if an exception occurred during registry cleanup (such as a timeout inside DoFn teardown), the pipeline state was transitioned to terminal before the exception was queued in `visibleUpdates`. This introduced a race condition where `waitUntilFinish()` could detect the terminal state and exit successfully before the exception was offered to the updates queue, swallowing the exception. This caused tests like `CallTest.givenTeardownTimeout_throwsError` to fail since they expected the pipeline to throw an exception. This change swaps the order so that the exception is posted to `visibleUpdates` before updating the pipeline state to terminal, ensuring the exception is always propagated.
1 parent 464546b commit 0d4efb1

1 file changed

Lines changed: 5 additions & 2 deletions

File tree

runners/direct-java/src/main/java/org/apache/beam/runners/direct/ExecutorServiceParallelExecutor.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,9 @@ private void shutdownIfNecessary(State newState) {
348348
} catch (final Exception e) {
349349
errors.add(e);
350350
}
351-
pipelineState.compareAndSet(State.RUNNING, newState); // ensure we hit a terminal node
351+
IllegalStateException exception = null;
352352
if (!errors.isEmpty()) {
353-
final IllegalStateException exception =
353+
exception =
354354
new IllegalStateException(
355355
"Error"
356356
+ (errors.size() == 1 ? "" : "s")
@@ -359,6 +359,9 @@ private void shutdownIfNecessary(State newState) {
359359
.map(Exception::getMessage)
360360
.collect(Collectors.joining("\n- ", "- ", "")));
361361
visibleUpdates.failed(exception);
362+
}
363+
pipelineState.compareAndSet(State.RUNNING, newState); // ensure we hit a terminal node
364+
if (exception != null) {
362365
throw exception;
363366
}
364367
}

0 commit comments

Comments
 (0)