Skip to content

[#504] Guard asyncValidator setting error with _statusChanges stream status#505

Open
LahaLuhem wants to merge 4 commits into
joanpablo:masterfrom
LahaLuhem:master
Open

[#504] Guard asyncValidator setting error with _statusChanges stream status#505
LahaLuhem wants to merge 4 commits into
joanpablo:masterfrom
LahaLuhem:master

Conversation

@LahaLuhem

@LahaLuhem LahaLuhem commented May 26, 2026

Copy link
Copy Markdown

Connection with issue(s)

Close #504

Solution description

The StateError: Cannot add new events after calling close from #504 is the visible symptom of an orphan-subscription race in _cancelExistingSubscription(). The same race can also cause stale validator results to overwrite newer ones, even without dispose().

Root cause

_cancelExistingSubscription() was async and called without await from updateValueAndValidity():

Future<void> _cancelExistingSubscription() async {
  await _asyncValidationSubscription?.cancel();
  _asyncValidationSubscription = null;
}
  1. The await suspends — even when _asyncValidationSubscription is null, await null still yields via a microtask.
  2. Control returns to updateValueAndValidity(), which synchronously runs _runAsyncValidators() and stores a fresh subscription in _asyncValidationSubscription.
  3. The deferred microtask resumes and runs _asyncValidationSubscription = null, orphaning the just-assigned subscription.
  4. The orphan has no reference, so neither subsequent value changes nor dispose() can cancel it. When it eventually completes, its onDone:
    • crashes if _statusChanges has been closed (the original Use-after-free bug possible for AsyncValidators #504 symptom), or
    • re-injects a stale error via setErrors(...).addAll(errors).addAll(asyncValidationErrors), overwriting whatever the latest validator settled on.

Fix

  1. Make _cancelExistingSubscription synchronous. StreamSubscription.cancel() stops event delivery synchronously; the returned future only signals cleanup completion, so awaiting it serves no purpose here and is what introduces the suspension point.
  2. Cancel _debounceTimer in dispose(). Without this, disposing a control while a debounce window is open leaks the timer (and the closure holding this).

With both changes the symptom-level if (_statusChanges.isClosed) return; guard is unnecessary — the orphan that produced the bad onDone can no longer be created.

Tests

test/src/validators/async_validator_test.dart:

  • Dispose during in-flight async validation — six tests covering dispose at various points during validation for FormControl / FormGroup, single and multiple validators, including the debounce window. Also asserts dispose() cancels the pending debounce timer via FakeAsync.pendingTimers.
  • Async validator subscription cancellation — sets value = 'bad' (slow validator, returns an error), then value = 'good' (fast, returns no error); asserts the stale 'bad' error does not survive into the 'good' state. Fails on the original code because the orphaned 'bad' subscription is never cancelled and re-injects its error after 'good' has already settled.

Each test was verified to fail when the corresponding part of the fix is reverted.

To Do

  • Check the original issue to confirm it is fully satisfied
  • Add solution description to help guide reviewers
  • Add unit test to verify new or fixed behaviour
  • If apply, add documentation to code properties and package readme

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds a check to prevent a crash when _statusChanges is closed during the onDone callback in AbstractControl. However, the review feedback points out that this only addresses the symptom of a larger issue. The root cause is a subscription leak caused by a race condition in the asynchronous _cancelExistingSubscription() method, which gets overwritten with null and orphaned. The reviewer recommends making _cancelExistingSubscription() synchronous and ensuring the _debounceTimer is cancelled in dispose() to properly resolve the leak.

Comment thread lib/src/models/models.dart Outdated
@LahaLuhem LahaLuhem changed the title [#504] Fix use-after-free bug for async validators [#504] Guard asyncValidator setting error with _statusChanges stream status May 26, 2026
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.

Use-after-free bug possible for AsyncValidators

1 participant