Skip to content

[release/11.0] Ensure async validation tasks are cleaned up during initialization - #132705

Merged
artl93 merged 1 commit into
release/11.0from
backport/pr-132685-to-release/11.0
Aug 25, 2026
Merged

[release/11.0] Ensure async validation tasks are cleaned up during initialization#132705
artl93 merged 1 commit into
release/11.0from
backport/pr-132685-to-release/11.0

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Backport of #132685 to release/11.0.

/cc @jeffhandley

Customer Impact

  • Customer reported
  • 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
  • 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.

…132685)

## 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.

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.

@jeffhandley jeffhandley added the Servicing-consider Issue for next servicing release review label Aug 24, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@artl93 I recommend this PR for .NET 11 RC2. The need for this change was identified during security review.

@artl93 artl93 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.

Quality. New scenario. Approved.

@jeffhandley jeffhandley added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 24, 2026
@tarekgh

tarekgh commented Aug 24, 2026

Copy link
Copy Markdown
Member

nit (non-blocking): the cancel-and-await cleanup is the right call. The one residual edge is a validator that ignores its CancellationToken, since after linkedCts.Cancel() we still await it, so a non-cooperative validator can delay both the exceptional path and the normal breakOnFirstError short-circuit. A timeout wouldn't help, it would just trade the hang for returning while a rogue task still uses caller-owned state.

Could we instead make the contract explicit as a docs-only follow-up? Add a remark on AsyncValidationAttribute.GetValidationResultAsync that implementations must observe the token, and that callers wanting an upper bound should pass one linked to a timeout (new CancellationTokenSource(timeout).Token). Not a blocker for the backport.

@ViveliDuCh ViveliDuCh self-assigned this Aug 24, 2026
@artl93
artl93 merged commit ab6dfe4 into release/11.0 Aug 25, 2026
98 of 101 checks passed
@artl93
artl93 deleted the backport/pr-132685-to-release/11.0 branch August 25, 2026 17:52
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.

4 participants