Guard the pane toggle against a disposed navigation service - #3458
Open
FrayxRulez wants to merge 1 commit into
Open
FrayxRulez wants to merge 1 commit into
FrayxRulez wants to merge 1 commit into
Conversation
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
NullReferenceException— "Object reference not set to an instance of an object.", reported by crash telemetry on 12.10.7.Cause
The null is
MasterDetail.NavigationService:It is null in exactly two states: before
MainPage.Initialize(), which runs inOnNavigatedToandso precedes any focus the page can receive, and after
MasterDetailView.Dispose(), whose laststatements are
NavigationService = null; ViewModel = null; DetailFrame = null;.It is the second one.
RootWindow.SwitchcallsDestroy(_navigationService)— which disposes theIRootContentPage, i.e.MainPage, and through itMasterDetail— before the new session'sframe replaces the old one in
Navigation.Content. In between, the disposed page is still theframe's content and still in the visual tree, still wired to its own handlers.
Navigation.IsPaneOpen = falseon the next line closes the accounts pane, and the focus that returns to the content landson the dead page; if it lands on
SearchField, itsGotFocus="Search_Click"runsSearch_TextChangedand the crash follows.The log tail of the reports ends on exactly that sequence:
ClearCache Frame: Main3,NavigateFrom Suspending: True, thenNavigationServiceFactory backButton: Attachfor the incomingsession —
Destroyfollowed by the new service being built.Search_LostFocus, twenty lines away, already reads the same expression asMasterDetail.NavigationService?.CurrentPageType, so the null is known there and just missing here.Change
The same
?., which resolvesvisibleto false — the pane toggle on a page that is being torn down.Worth noting separately: the page stays fully reactive after
Dispose(), andMainPagehas aroundthirty other unguarded
MasterDetail.NavigationService.dereferences that the same window exposes.Ordering
Navigation.IsPaneOpen = falsebeforeDestroyinRootWindow.Switchwould narrow it butnot close it, since focus restoration is not synchronous. Neither is changed here.
Verification
Not built — a UWP/.NET Native build is not available here. The file was parsed with Roslyn
(
CSharpSyntaxTree.ParseText), which confirms it still parses but says nothing about types.