(1.9 port) Fix a number of shutdown crashes in TermControl (#10117)

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 661fde5937)
This commit is contained in:
Dustin L. Howett 2021-05-18 17:36:40 -05:00 committed by GitHub
parent 996a680ec3
commit 3a27386e25
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 16 deletions

View file

@ -134,8 +134,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Return Value:
// - <none>
void TSFInputControl::TryRedrawCanvas()
try
{
if (!_focused)
if (!_focused || !Canvas())
{
return;
}
@ -164,6 +165,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_RedrawCanvas();
}
CATCH_LOG()
// Method Description:
// - Redraw the Canvas and update the current Text Bounds and Control Bounds for

View file

@ -535,7 +535,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get());
_core->AttachUiaEngine(_uiaEngine.get());
return *autoPeer;
_automationPeer = *autoPeer;
return _automationPeer;
}
return nullptr;
}
@ -1719,6 +1720,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_RestorePointerCursorHandlers(*this, nullptr);
// These four throttled functions are triggered by terminal output and interact with the UI.
// Since Close() is the point after which we are removed from the UI, but before the destructor
// has run, we should disconnect them *right now*. If we don't, they may fire between the
// throttle delay (from the final output) and the dtor.
_tsfTryRedrawCanvas.reset();
_updatePatternLocations.reset();
_updateScrollBar.reset();
_playWarningBell.reset();
// Disconnect the TSF input control so it doesn't receive EditContext events.
TSFInputControl().Close();
_autoScrollTimer.Stop();

View file

@ -138,6 +138,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// in order to safely resolve this unsafe pointer dependency. Otherwise a deallocated
// IRenderEngine is accessed when ControlCore calls Renderer::TriggerTeardown.
// (C++ class members are destroyed in reverse order.)
// Further, the TermControlAutomationPeer must be destructed after _uiaEngine!
winrt::Windows::UI::Xaml::Automation::Peers::AutomationPeer _automationPeer{ nullptr };
std::unique_ptr<::Microsoft::Console::Render::UiaEngine> _uiaEngine;
winrt::com_ptr<ControlCore> _core;

View file

@ -46,9 +46,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TermControlAutomationPeer::SignalSelectionChanged()
{
UiaTracing::Signal::SelectionChanged();
Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() {
// The event that is raised when the text selection is modified.
RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged);
auto dispatcher{ Dispatcher() };
if (!dispatcher)
{
return;
}
dispatcher.RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [weakThis{ get_weak() }]() {
if (auto strongThis{ weakThis.get() })
{
// The event that is raised when the text selection is modified.
strongThis->RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged);
}
});
}
@ -61,9 +69,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TermControlAutomationPeer::SignalTextChanged()
{
UiaTracing::Signal::TextChanged();
Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() {
// The event that is raised when textual content is modified.
RaiseAutomationEvent(AutomationEvents::TextPatternOnTextChanged);
auto dispatcher{ Dispatcher() };
if (!dispatcher)
{
return;
}
dispatcher.RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [weakThis{ get_weak() }]() {
if (auto strongThis{ weakThis.get() })
{
// The event that is raised when textual content is modified.
strongThis->RaiseAutomationEvent(AutomationEvents::TextPatternOnTextChanged);
}
});
}
@ -76,14 +92,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TermControlAutomationPeer::SignalCursorChanged()
{
UiaTracing::Signal::CursorChanged();
Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() {
// The event that is raised when the text was changed in an edit control.
// Do NOT fire a TextEditTextChanged. Generally, an app on the other side
// will expect more information. Though you can dispatch that event
// on its own, it may result in a nullptr exception on the other side
// because no additional information was provided. Crashing the screen
// reader.
RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged);
auto dispatcher{ Dispatcher() };
if (!dispatcher)
{
return;
}
dispatcher.RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [weakThis{ get_weak() }]() {
if (auto strongThis{ weakThis.get() })
{
// The event that is raised when the text was changed in an edit control.
// Do NOT fire a TextEditTextChanged. Generally, an app on the other side
// will expect more information. Though you can dispatch that event
// on its own, it may result in a nullptr exception on the other side
// because no additional information was provided. Crashing the screen
// reader.
strongThis->RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged);
}
});
}