Skip to content

Ensure async validation tasks are cleaned up during initialization - #132685

Merged
ViveliDuCh merged 1 commit into
mainfrom
fix-dataannotations-async-task-cleanup
Aug 24, 2026
Merged

Ensure async validation tasks are cleaned up during initialization#132685
ViveliDuCh merged 1 commit into
mainfrom
fix-dataannotations-async-task-cleanup

Conversation

@ViveliDuCh

Copy link
Copy Markdown
Member

Summary

  • Move async validator task creation inside existing cleanup scopes.
  • Cancel and await partially started tasks if setup fails.
  • Clarify comments describing task-lifetime guarantees.

Testing

  • System.ComponentModel.Annotations build passed.
  • System.ComponentModel.Annotations.Tests: 984 passed, 0 failed.

Note

This description was generated by GitHub Copilot.

Move task initialization inside the existing try/finally scopes so partially started validators are canceled and awaited if setup throws.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16be2aa5-27c1-4d7c-8a93-1e5056fed9b9
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

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.

Pull request overview

This PR tightens the lifetime management of asynchronous validation tasks in Validator so that partially started validations are canceled and awaited on failure/early-exit paths, preventing leftover background work from escaping the validation call.

Changes:

  • Move async validation task creation into existing try/finally cleanup scopes.
  • Ensure remaining in-flight tasks are canceled and awaited when exiting early (including exceptions during task setup).
  • Update comments to reflect the stronger task-lifetime/cleanup guarantee.

@Youssef1313 Youssef1313 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ViveliDuCh
ViveliDuCh merged commit 04e13d5 into main Aug 24, 2026
81 checks passed
@ViveliDuCh
ViveliDuCh deleted the fix-dataannotations-async-task-cleanup branch August 24, 2026 15:22
@ViveliDuCh

Copy link
Copy Markdown
Member Author

/backport to release/11.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0 (link to workflow run)

@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Aug 25, 2026
artl93 pushed a commit that referenced this pull request Aug 25, 2026
…itialization (#132705)

Backport of #132685 to `release/11.0`.

/cc @jeffhandley

## Customer Impact

- [ ] Customer reported
- [x] Found internally

Async DataAnnotations validation is new in .NET 11. During task fan-out,
setup could fail after some validation tasks had started but before
execution entered the `try`/`finally` cleanup scope.

Those tasks could outlive the public `Validator` call and continue using
the caller-owned object, `ValidationContext`, or services after the
caller resumed or disposed them. This leaves the new async-validation
scenario significantly incomplete and creates a reliability risk from
escaped background work and resource-lifetime races.

The fix moves task creation inside the existing cleanup scopes, ensuring
every started task is cancelled and awaited before control returns.

This meets the .NET 11 bug bar as:

- Changes needed based on threat modeling / security review.
- A broken or significantly incomplete scenario for a feature new in the
release.
- A significant reliability issue.

## Regression

- [ ] Yes
- [x] No

This is not a regression from .NET 10 because async DataAnnotations
validation was introduced in .NET 11 by #128656. It completes the
lifetime guarantees of that new feature.

## Testing

On `main`:

- `System.ComponentModel.Annotations` built successfully.
- `System.ComponentModel.Annotations.Tests` passed: 984 passed, 0
failed.

Existing tests cover parallel async validation, cancellation,
short-circuiting, and result collection. No targeted test was added
because the protected path requires a synchronous failure during
internal fan-out setup, which cannot be injected deterministically
through the public API without white-box manipulation.

The same component tests and PR CI validate the backport on
`release/11.0`.

## Risk

Low. The change modifies one private implementation file, adds no API,
and does not alter normal validation results or task ordering. It only
extends the existing cancellation-and-await cleanup boundary to cover
failures during task setup.

On exceptional setup paths, the method may wait for a non-cooperative
validator before propagating the original exception. This is intentional
because returning while validation still uses caller-owned state is the
reliability problem being fixed.

Co-authored-by: Viviana Dueñas <50237907+ViveliDuCh@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16be2aa5-27c1-4d7c-8a93-1e5056fed9b9
artl93 pushed a commit that referenced this pull request Aug 28, 2026
Backport of #132794 to release/11.0

/cc @jeffhandley @tarekgh

## Customer Impact

- [ ] Customer reported
- [x] Found internally

Async DataAnnotations validation is new in .NET 11 (#128656). The
reliability fix #132685, backported for RC2 as #132705, moves validation
task creation inside the cancel-and-await cleanup scope so no task can
outlive the public `Validator` call and keep using caller-owned state.

Review of that backport surfaced one residual edge that cannot be fixed
in code: a validator that ignores its `CancellationToken`. Because
cancellation is cooperative, cleanup must still await such a validator,
and a timeout would only trade the stall for returning while a rogue
task still uses caller-owned state. The agreed resolution was to
document the contract. This PR adds remarks on
`AsyncValidationAttribute` that implementations must observe the token,
and that callers wanting a time bound should pass one linked to a
timeout. Without it, the new scenario's cancellation contract ships
significantly incomplete in the .NET 11 API docs. The underlying need
was identified during security review.

This meets the .NET 11 bug bar as:

- A significantly incomplete scenario for a feature new in the release —
the documented cancellation contract for async DataAnnotations
validation.
- A reliability guarantee: the documented contract is what stops
implementers from reintroducing the escaped-background-work race that
#132705 fixed.

## Regression

- [ ] Yes
- [x] No

Not a regression. Async DataAnnotations validation is new in .NET 11
(#128656); this completes that new feature's documented contract rather
than restoring prior behavior.

## Testing

Documentation-only change: XML doc comment remarks on
`AsyncValidationAttribute`. No API surface, IL, or runtime behavior
changes, so there is nothing to unit-test and no new tests were added.
Verified by building `System.ComponentModel.Annotations` so the doc
comments and any `<see cref>` references resolve. Existing
`System.ComponentModel.Annotations.Tests` are unaffected.

## Risk

Minimal. The change touches only XML documentation comments in one file.
It adds no API, no code, no IL, and does not alter validation results,
task ordering, or cancellation behavior. The only review surface is
wording accuracy.

> [!NOTE]
> This pull request description was generated with GitHub Copilot.

Co-authored-by: Viviana Dueñas <50237907+ViveliDuCh@users.noreply.github.com>
Copilot-Session: c5c574e4-16c6-4f14-9125-3cd4120763df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants