Skip to content

fix(ui_auth, ui_storage): don't call setState after dispose in ProfileScreen, DeleteAccountButton and UploadButton - #721

Open
BurningAXE wants to merge 2 commits into
firebase:mainfrom
BurningAXE:fix/setstate-after-dispose-profile-delete-upload
Open

BurningAXE wants to merge 2 commits into
firebase:mainfrom
BurningAXE:fix/setstate-after-dispose-profile-delete-upload

Conversation

@BurningAXE

@BurningAXE BurningAXE commented Oct 12, 2026 •

Copy link
Copy Markdown

Description

Follow-up to #720, which fixed the same bug in ForgotPasswordView.

Several widgets await a Firebase call or a dialog and then call setState without checking mounted, often in a finally block. If the widget leaves the tree while the await is pending (the screen is popped, or a parent rebuilds without it), that setState runs on a disposed State:

  • Debug: setState() called after dispose(): …
  • Release: the assertion is compiled out, so State.setState reaches _element!.markNeedsBuild() and throws Null check operator used on a null value.

Affected sites:

Widget After which await
DeleteAccountButton currentUser.delete() (finally). The existing if (!mounted) return; inside try doesn't skip finally, so the crash still happens on the success path
DeleteAccountButton the confirmation dialog / deleteConfirmation, and the retry after onSignInRequired. After a requires-recent-login error it also called onSignInRequired itself, and ProfileScreen's implementation looks up localizations with its own defunct context
ProfileScreen (linked providers) unlink() + reload(), in the success path, catch and finally
ProfileScreen (MFA toggle, disable) getEnrolledFactors(), and unenroll() in the success path, catch and finally. On a recent-login error it also opened the re-authentication dialog with the disposed context, then retried _disable()
ProfileScreen (MFA toggle, enable) enroll() in the CredentialReceived action (success path, catch, finally), and the final setState after startPhoneVerification returns. The finally also pops until the profile screen's route; if that route has been removed, popUntil never finds it and pops every route, so it now checks currentRoute.isActive first
UploadButton (firebase_ui_storage) the upload task (finally)

The fix uses the usual pattern:

  • After an await, if (!mounted) return; before touching the State. If the widget is gone before a follow-up step starts, that step is skipped: deleting after confirmation, unenrolling after the factor lookup, asking to re-authenticate (onSignInRequired, the MFA re-authentication dialog), retrying, or rebuilding through onToggled / onProviderUnlinked. Those rebuild callbacks target a RebuildScope that went away with the widget.
  • setState inside catch / finally is wrapped in if (mounted). The rest of those blocks still runs. For example, the MFA enrolment finally still pops the phone verification route, and UploadButton still calls onUploadComplete / onError.

Tests

  • test/widgets/delete_account_button_test.dart (new, 4 tests)
  • test/screens/profile_screen_test.dart (new, 12 tests)
  • firebase_ui_storage/test/upload_button_test.dart (new, 1 test; skipped on web because XFile.readAsBytes can't be faked through the file_selector method channel)

Each test disposes the widget while the await is pending, then completes it. All 17 fail on main and pass with this change. Most fail with setState() called after dispose() or an equivalent error, and the rest because the follow-up step still ran. I also turned each of the 17 new mounted checks, and the isActive check, off one at a time, and every one made at least one test fail.

ProfileScreen tests dispose the MFA toggle under an open dialog / phone verification route by re-pumping the screen with showMFATile: false, so the route stack (and popUntil) stays intact.

Related Issues

None found. #720 fixed the same pattern in ForgotPasswordView. #353 was another case of it, in EmailVerificationBadge. Open PR #279 touches DeleteAccountButton and ProfileScreen too, but in different lines.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///). (n/a: no public API change)
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • All unit tests pass (melos run test:unit:all doesn't fail).
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

🤖 Generated with Claude Code

…eScreen, DeleteAccountButton and UploadButton

Several widgets await a Firebase call or a dialog and then call `setState`
(often in a `finally` block) without checking `mounted`. If the widget is
removed from the tree while the await is pending, e.g. the screen is popped
or the parent rebuilds without it, the `setState` runs on a defunct State:
"setState() called after dispose()" in debug builds and "Null check operator
used on a null value" from `State.setState` in release builds.

- DeleteAccountButton: `finally` after deleting the account. The existing
  `if (!mounted) return;` in the `try` block does not skip `finally`. Also
  the `setState` after the confirmation dialog and after re-authentication.
- ProfileScreen, linked providers: `setState` after unlinking a provider, in
  its `catch` and in its `finally`.
- ProfileScreen, MFA toggle: after loading enrolled factors, after
  unenrolling (plus re-authenticating with a defunct context), after
  enrolling, and after phone verification returns.
- UploadButton: `finally` after the upload completes.

After each await, bail out with `if (!mounted) return;` before touching the
State. Wrap `setState` in `catch`/`finally` blocks in `if (mounted)`. Each
guard has a regression test that disposes the widget while the await is
pending.

@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 introduces mounted checks across several stateful widgets (_LinkedProvidersRowState, _MFAToggleState, _DeleteAccountButtonState, and _UploadButtonState) to prevent calling setState or executing callbacks after the widgets have been disposed. It also adds comprehensive unit tests to verify that these widgets handle unmounting gracefully during asynchronous operations. The review feedback suggests two important safety improvements: adding a mounted check in delete_account_button.dart before invoking onSignInRequired and the recursive _deleteAccount call, and checking mounted in profile_screen.dart before popping the navigation stack to avoid potential crashes if the route is no longer present.

Comment thread packages/firebase_ui_auth/lib/src/screens/profile_screen.dart
DeleteAccountButton called onSignInRequired after delete() failed with
requires-recent-login even if it had been disposed in the meantime.
ProfileScreen's onSignInRequired then looked up localizations with its
own defunct context and threw.

The MFA enrolment finally block popped until the profile screen's route.
If that route had been removed under the phone verification screen,
popUntil never found it and popped every route.
@BurningAXE
BurningAXE marked this pull request as ready for review October 13, 2026 11:04
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.

1 participant