Repository navigation
Bug 2080456 - AsyncShutdown timeout crash in "RemoteSettingsExperimentLoader: disabling" when a recipe update hangs (e.g. offline) - #383
Closed
mrwiora wants to merge 1 commit into
Conversation
…ttingsExperimentLoader. r?#nimbus-reviewers The appShutdownConfirmed blocker waits for any in-progress update to finish, but recordTargetingContext() could not be interrupted, so a hang there turned into an AsyncShutdown timeout crash. Claude-Session: https://claude.ai/code/session_0177zRiHaA8dyT6BqNxv4bo4
No Taskcluster jobs started for this pull requestThe |
Contributor
|
(Automated Close) Please do not file pull requests here, see https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Firefox crashes currently in poorly configured offline environments (where packages are not rejected, but silently dropped). As this might happen more often than expected, I would like to bring in a fix.
Analysis: The shutdown blocker aborts the loader's AbortController and then awaits finishedUpdating(), which resolves only when the in-progress update finishes. Only some steps of #updateImpl() react to the abort: they are wrapped in #raceShutdown() (RemoteSettingsClient.get(), NimbusEnrollments.loadUnenrolledExperimentSlugsFromOtherProfiles()) or guarded by withUpdateLock() / #throwDuringShutdown(). The call to recordTargetingContext() at the start of #updateImpl() is awaited directly. It evaluates every targeting attribute (bookmarks, FxA/Sync state, add-ons, default browser, profile group, ...) and then awaits ClientID.getProfileGroupID(). If any of these does not settle, the update never finishes, finishedUpdating() never resolves, and the blocker hits the AsyncShutdown crash timeout.
On a profile that has never synced, there is no packaged dump for nimbus-desktop-experiments, so RemoteSettingsClient.get() goes to the network. Builds that do not yet wrap that call in #raceShutdown() hang there as well in an offline environment.
Proposed fix: Wrap recordTargetingContext() in #raceShutdown(), the same way the other interruptible steps of #updateImpl() are handled. Targeting context recording is telemetry only, so abandoning it at shutdown loses nothing persistent. An xpcshell test that follows the existing tests in toolkit/components/nimbus/test/unit/shutdown/ is included.
https://bugzilla.mozilla.org/show_bug.cgi?id=2080456
Lando: link
Bugzilla: bug 2080456
🚫 This pull request has 2 blockers.