Skip to content

Guard the pane toggle against a disposed navigation service - #3458

Open
FrayxRulez wants to merge 1 commit into
developfrom
pane-toggle-after-dispose
Open

FrayxRulez wants to merge 1 commit into
developfrom
pane-toggle-after-dispose

Conversation

@FrayxRulez

Copy link
Copy Markdown
Collaborator

NullReferenceException — "Object reference not set to an instance of an object.", reported by crash telemetry on 12.10.7.

   0  Telegram.Views.MainPage.UpdatePaneToggleButtonVisibility+0x77
      Telegram\Views\MainPage.xaml.cs:1989
   1  Telegram.Views.MainPage.Search_TextChanged+0x15f
      Telegram\Views\MainPage.xaml.cs:2478
   2  Telegram.Views.MainPage.Search_Click+0x3d
      Telegram\Views\MainPage.xaml.cs:2432

Cause

The null is MasterDetail.NavigationService:

if (MasterDetail.CurrentState == MasterDetailState.Minimal)
{
    visible &= MasterDetail.NavigationService.CurrentPageType == typeof(BlankPage);
}

It is null in exactly two states: before MainPage.Initialize(), which runs in OnNavigatedTo and
so precedes any focus the page can receive, and after MasterDetailView.Dispose(), whose last
statements are NavigationService = null; ViewModel = null; DetailFrame = null;.

It is the second one. RootWindow.Switch calls Destroy(_navigationService) — which disposes the
IRootContentPage, i.e. MainPage, and through it MasterDetail — before the new session's
frame replaces the old one in Navigation.Content. In between, the disposed page is still the
frame's content and still in the visual tree, still wired to its own handlers. Navigation.IsPaneOpen = false on the next line closes the accounts pane, and the focus that returns to the content lands
on the dead page; if it lands on SearchField, its GotFocus="Search_Click" runs
Search_TextChanged and the crash follows.

The log tail of the reports ends on exactly that sequence: ClearCache Frame: Main3,
NavigateFrom Suspending: True, then NavigationServiceFactory backButton: Attach for the incoming
session — Destroy followed by the new service being built.

Search_LostFocus, twenty lines away, already reads the same expression as
MasterDetail.NavigationService?.CurrentPageType, so the null is known there and just missing here.

Change

The same ?., which resolves visible to false — the pane toggle on a page that is being torn down.

Worth noting separately: the page stays fully reactive after Dispose(), and MainPage has around
thirty other unguarded MasterDetail.NavigationService. dereferences that the same window exposes.
Ordering Navigation.IsPaneOpen = false before Destroy in RootWindow.Switch would narrow it but
not 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.

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