Try to fix crash on close with saving enabled (#11440)

Don't crash if we try to save the window layout while we are closing, and try to avoid saving at all.

Might impact #11354 

## Detailed Description of the Pull Request / Additional comments
- Revoke the event handler/save throttler so we don't even try to get the window layout when we are closing
- Try to check for nullptrs, but then apply `try {} CATCH_LOG()` liberally

## Validation Steps Performed
The happy path of saving normally is still fine, but I haven't been unlucky enough to trigger the crash myself.
This commit is contained in:
Schuyler Rosefield 2021-10-19 20:12:18 -04:00 committed by GitHub
parent 5a23029dac
commit 6bf1507a6c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 14 deletions

View File

@ -86,7 +86,7 @@ AppHost::AppHost() noexcept :
_window->SetAlwaysOnTop(_logic.GetInitialAlwaysOnTop());
_window->MakeWindow();
_windowManager.GetWindowLayoutRequested([this](auto&&, const winrt::Microsoft::Terminal::Remoting::GetWindowLayoutArgs& args) {
_GetWindowLayoutRequestedToken = _windowManager.GetWindowLayoutRequested([this](auto&&, const winrt::Microsoft::Terminal::Remoting::GetWindowLayoutArgs& args) {
// The peasants are running on separate threads, so they'll need to
// swap what context they are in to the ui thread to get the actual layout.
args.WindowLayoutJsonAsync(_GetWindowLayoutAsync());
@ -401,23 +401,42 @@ void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*se
_HideNotificationIconRequested();
}
// We don't want to try to save layouts if we are about to close
_getWindowLayoutThrottler.reset();
_windowManager.GetWindowLayoutRequested(_GetWindowLayoutRequestedToken);
// Remove ourself from the list of peasants so that we aren't included in
// any future requests. This will also mean we block until any existing
// event handler finishes.
_windowManager.SignalClose();
_window->Close();
}
LaunchPosition AppHost::_GetWindowLaunchPosition()
{
// Get the position of the current window. This includes the
// non-client already.
const auto window = _window->GetWindowRect();
const auto dpi = _window->GetCurrentDpi();
const auto nonClientArea = _window->GetNonClientFrame(dpi);
// The nonClientArea adjustment is negative, so subtract that out.
// This way we save the user-visible location of the terminal.
LaunchPosition pos{};
pos.X = window.left - nonClientArea.left;
pos.Y = window.top;
// If we started saving before closing, but didn't resume the event handler
// until after _window might be a nullptr.
if (!_window)
{
return pos;
}
try
{
// Get the position of the current window. This includes the
// non-client already.
const auto window = _window->GetWindowRect();
const auto dpi = _window->GetCurrentDpi();
const auto nonClientArea = _window->GetNonClientFrame(dpi);
// The nonClientArea adjustment is negative, so subtract that out.
// This way we save the user-visible location of the terminal.
pos.X = window.left - nonClientArea.left;
pos.Y = window.top;
}
CATCH_LOG();
return pos;
}
@ -723,10 +742,15 @@ winrt::Windows::Foundation::IAsyncOperation<winrt::hstring> AppHost::_GetWindowL
{
winrt::apartment_context peasant_thread;
winrt::hstring layoutJson = L"";
// Use the main thread since we are accessing controls.
co_await winrt::resume_foreground(_logic.GetRoot().Dispatcher());
const auto pos = _GetWindowLaunchPosition();
const auto layoutJson = _logic.GetWindowLayoutJson(pos);
try
{
const auto pos = _GetWindowLaunchPosition();
layoutJson = _logic.GetWindowLayoutJson(pos);
}
CATCH_LOG()
// go back to give the result to the peasant.
co_await peasant_thread;

View File

@ -114,4 +114,5 @@ private:
winrt::event_token _NotificationIconPressedToken;
winrt::event_token _ShowNotificationIconContextMenuToken;
winrt::event_token _NotificationIconMenuItemSelectedToken;
winrt::event_token _GetWindowLayoutRequestedToken;
};