Repository navigation
fix(ui_auth, ui_storage): don't call setState after dispose in ProfileScreen, DeleteAccountButton and UploadButton - #721
Conversation
…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.
There was a problem hiding this comment.
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.
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.
Description
Follow-up to #720, which fixed the same bug in
ForgotPasswordView.Several widgets
awaita Firebase call or a dialog and then callsetStatewithout checkingmounted, often in afinallyblock. If the widget leaves the tree while theawaitis pending (the screen is popped, or a parent rebuilds without it), thatsetStateruns on a disposedState:setState() called after dispose(): …State.setStatereaches_element!.markNeedsBuild()and throwsNull check operator used on a null value.Affected sites:
awaitDeleteAccountButtoncurrentUser.delete()(finally). The existingif (!mounted) return;insidetrydoesn't skipfinally, so the crash still happens on the success pathDeleteAccountButtondeleteConfirmation, and the retry afteronSignInRequired. After arequires-recent-loginerror it also calledonSignInRequireditself, andProfileScreen's implementation looks up localizations with its own defunctcontextProfileScreen(linked providers)unlink()+reload(), in the success path,catchandfinallyProfileScreen(MFA toggle, disable)getEnrolledFactors(), andunenroll()in the success path,catchandfinally. On a recent-login error it also opened the re-authentication dialog with the disposedcontext, then retried_disable()ProfileScreen(MFA toggle, enable)enroll()in theCredentialReceivedaction (success path,catch,finally), and the finalsetStateafterstartPhoneVerificationreturns. Thefinallyalso pops until the profile screen's route; if that route has been removed,popUntilnever finds it and pops every route, so it now checkscurrentRoute.isActivefirstUploadButton(firebase_ui_storage)finally)The fix uses the usual pattern:
await,if (!mounted) return;before touching theState. 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 throughonToggled/onProviderUnlinked. Those rebuild callbacks target aRebuildScopethat went away with the widget.setStateinsidecatch/finallyis wrapped inif (mounted). The rest of those blocks still runs. For example, the MFA enrolmentfinallystill pops the phone verification route, andUploadButtonstill callsonUploadComplete/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 becauseXFile.readAsBytescan't be faked through thefile_selectormethod channel)Each test disposes the widget while the
awaitis pending, then completes it. All 17 fail onmainand pass with this change. Most fail withsetState() called after dispose()or an equivalent error, and the rest because the follow-up step still ran. I also turned each of the 17 newmountedchecks, and theisActivecheck, off one at a time, and every one made at least one test fail.ProfileScreentests dispose the MFA toggle under an open dialog / phone verification route by re-pumping the screen withshowMFATile: false, so the route stack (andpopUntil) stays intact.Related Issues
None found. #720 fixed the same pattern in
ForgotPasswordView. #353 was another case of it, inEmailVerificationBadge. Open PR #279 touchesDeleteAccountButtonandProfileScreentoo, but in different lines.Checklist
///). (n/a: no public API change)melos run analyze) does not report any problems on my PR.melos run test:unit:alldoesn't fail).Breaking Change
🤖 Generated with Claude Code