Skip to content

Fix a number of shutdown crashes in TermControl#10115

Merged
DHowett merged 1 commit intorelease-1.7from
dev/duhowett/1.7-stab
May 17, 2021
Merged

Fix a number of shutdown crashes in TermControl#10115
DHowett merged 1 commit intorelease-1.7from
dev/duhowett/1.7-stab

Conversation

@DHowett
Copy link
Member

@DHowett DHowett commented May 17, 2021

  1. The TSFInputControl may get a layout event after it has been removed
    from service (and no longer has a XAML tree)
    • Two fixes:
      • first, guard the layour updater from accessing detached xaml
        objects
      • second, shut down all pending throttled functions during close
        (not destruction!¹)
  2. The TermControlAutomationPeer may be destructed before its events
    fire.
  3. The TermControlAutomationPeer may receive a notification after it has
    been detached from XAML (and therefore has no dispatcher).

¹ Close happens before the control is removed from the XAML tree;
destruction happens some time later. We must detach all UI-bound events
in Close so that they don't fire between when we detach and when we
destruct.

Fixes MSFT-32496693
Fixes MSFT-32496158
Fixes MSFT-32509759
Fixes MSFT-32871913

1. The TSFInputControl may get a layout event after it has been removed
   from service (and no longer has a XAML tree)
   * Two fixes:
      * first, guard the layour updater from accessing detached xaml
	objects
      * second, shut down all pending throttled functions during close
	(not destruction!¹)
2. The TermControlAutomationPeer may be destructed before its events
   fire.
3. The TermControlAutomationPeer may receive a notification after it has
   been detached from XAML (and therefore has no dispatcher).

¹ Close happens before the control is removed from the XAML tree;
destruction happens some time later. We must detach all UI-bound events
in Close so that they don't fire between when we detach and when we
destruct.
@DHowett DHowett requested review from lhecker and zadjii-msft May 17, 2021 17:50
@DHowett
Copy link
Member Author

DHowett commented May 17, 2021

I will port this forward to 1.8, but I will need some assistance in porting it for 1.9.

@DHowett
Copy link
Member Author

DHowett commented May 17, 2021

In general (/cc @zadjii-msft), we need to be very crisp on what constitutes "teardown" for UI objects and teardown for the control. All of these crashes are caused by the UI being in a torn down state, but the control itself not being a torn down state and trying to manipulate the UI.

@DHowett
Copy link
Member Author

DHowett commented May 17, 2021

These account for 65% of all the WT Stable crashes in Watson (!)

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof merging this with #10051 and the rest of the branches in #5000 is gonna be fun

@zadjii-msft zadjii-msft added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels May 17, 2021
Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand how there cannot be a Dispatcher sometimes... I always assumed that it's valid until after the class was deallocated...

@DHowett DHowett merged commit 661fde5 into release-1.7 May 17, 2021
@DHowett DHowett deleted the dev/duhowett/1.7-stab branch May 17, 2021 19:22
DHowett added a commit that referenced this pull request May 17, 2021
1. The TSFInputControl may get a layout event after it has been removed
   from service (and no longer has a XAML tree)
   * Two fixes:
      * first, guard the layour updater from accessing detached xaml
	objects
      * second, shut down all pending throttled functions during close
	(not destruction!¹)
2. The TermControlAutomationPeer may be destructed before its events
   fire.
3. The TermControlAutomationPeer may receive a notification after it has
   been detached from XAML (and therefore has no dispatcher).

¹ Close happens before the control is removed from the XAML tree;
destruction happens some time later. We must detach all UI-bound events
in Close so that they don't fire between when we detach and when we
destruct.

Fixes MSFT-32496693
Fixes MSFT-32496158
Fixes MSFT-32509759
Fixes MSFT-32871913

(cherry picked from commit 661fde5)
@ghost
Copy link

ghost commented May 25, 2021

🎉Windows Terminal v1.8.1444.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented May 25, 2021

🎉Windows Terminal Preview v1.9.1445.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants