Skip to content

Use ConfigureAwait(false) on await foreach in ExtractorBase and TransformerBase - #363

Merged
Chris-Wolfgang merged 1 commit into
chore/fold-testkitfrom
fix/configureawait-await-foreach
Aug 13, 2026
Merged

Chris-Wolfgang merged 1 commit into
chore/fold-testkitfrom
fix/configureawait-await-foreach

Conversation

@Chris-Wolfgang

Copy link
Copy Markdown
Owner

Use ConfigureAwait(false) on await foreach in ExtractorBase / TransformerBase

Stacked on #357. Found during the 0.22.0 code-review pass.

The defect

Four await foreach sites resume on the caller's captured synchronization context:

Site
ExtractorBase.ExtractAsync ExtractorBase.cs:326
ExtractorBase.ExtractWithProgressAsync ExtractorBase.cs:344
TransformerBase.TransformAsync TransformerBase.cs:334
TransformerBase.TransformWithProgressAsync TransformerBase.cs:352

These packages ship net462 and netstandard2.0, where that is a real deadlock risk for any consumer calling sync-over-async. And because these are the two base classes every ETL stage inherits, the exposure is the whole family, not one code path.

Why CI never caught it

CA2007 is warning for src (.editorconfig:192) — but it does not analyse await foreach at all. The analyzer gate is structurally blind here, so this can only be found by reading.

Why this is a fix, not a style change

The rest of the package already does it — EtlPipelineImpl.cs:142, EtlPipelineSink.cs:59, MiddlewareExtensions.cs:56 and :121. These four were inconsistent with the package's own established convention, not a deliberate exception.

CHANGELOG

The 0.22.0 entry asserted the release was "not a behavioural change". That is no longer strictly true, so the entry is amended with a ### Fixed section rather than left to mislead.

Verification

Full solution build 0 warnings / 0 errors; 1085 tests green on net10.0.

…formerBase

Four await foreach sites resumed on the captured synchronization context:

  ExtractorBase.ExtractAsync            (ExtractorBase.cs:326)
  ExtractorBase.ExtractWithProgressAsync (ExtractorBase.cs:344)
  TransformerBase.TransformAsync         (TransformerBase.cs:334)
  TransformerBase.TransformWithProgressAsync (TransformerBase.cs:352)

These packages ship net462 and netstandard2.0, where resuming on the caller's
context is a real deadlock risk for consumers calling sync-over-async.

CA2007 is set to warning for src, but it does not analyse await foreach, so the
analyzer gate could never catch this. The rest of the package already used
ConfigureAwait(false) at every other await foreach (EtlPipelineImpl.cs:142,
EtlPipelineSink.cs:59, MiddlewareExtensions.cs:56 and :121) - these four were
inconsistent with the package's own convention rather than a deliberate choice.

The 0.22.0 CHANGELOG entry claimed the release was "not a behavioural change";
amended to note this fix rather than leave the claim inaccurate.

Full solution build 0 warnings / 0 errors; 1085 tests green on net10.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 13, 2026 11:58

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Chris-Wolfgang
Chris-Wolfgang merged commit 76518b9 into chore/fold-testkit Aug 13, 2026
2 checks passed
@Chris-Wolfgang
Chris-Wolfgang deleted the fix/configureawait-await-foreach branch August 13, 2026 12:05
Chris-Wolfgang added a commit that referenced this pull request Aug 13, 2026
)

Coyote 1.7.11 throws a NullReferenceException inside its own
CoyoteRuntime.IsTaskUncontrolled when it meets the
ConfiguredCancelableAsyncEnumerable awaiter introduced by the
ConfigureAwait(false) fix (#363) in ExtractorBase.ExtractWithResetAsync.

This is an instrumentation crash, not a discovered race: the run reports
"Found 1 bug" on iteration #1 having explored exactly 1 execution path in
0.096 sec. Coyote is dormant -- 1.7.11 is the newest release on nuget.org
and the last upstream commit was 2024-12-11 -- so there is no version to
upgrade to.

Reverting #363 was rejected: that would reintroduce a real net462 /
netstandard2.0 sync-over-async deadlock risk in the two base classes every
extract and transform stage inherits, purely to satisfy a test harness.

The test races DisposeAsync against an in-flight enumeration, so it
necessarily routes through ExtractAsync -> ExtractWithResetAsync; there is
no partial carve-out that keeps the test without the configured-enumerable
path. The method is left intact in the source file so restoring it is a
one-line change once #364 lands.

Concurrent_item_count_increments_never_lose_an_update is unaffected and
keeps running -- it never enumerates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants