From 48b20de4f4279cdffea710fcd1b456cf6c0766af Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 30 Sep 2021 11:15:35 -0500 Subject: [PATCH 1/2] Add some logging. Can't seem to get the crash to repro? --- .../ApplicationState.cpp | 12 +++++++ src/cascadia/WindowsTerminal/AppHost.cpp | 32 +++++++++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp index acd876884..3023789af 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp @@ -102,9 +102,21 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // The destructor ensures that the last write is flushed to disk before returning. ApplicationState::~ApplicationState() { + TraceLoggingWrite(g_hSettingsModelProvider, + "ApplicationState_Dtor_Start", + TraceLoggingDescription("Event at the start of the ApplicationState destructor"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // This will ensure that we not just cancel the last outstanding timer, // but instead force it to run as soon as possible and wait for it to complete. _throttler.flush(); + + TraceLoggingWrite(g_hSettingsModelProvider, + "ApplicationState_Dtor_End", + TraceLoggingDescription("Event at the end of the ApplicationState destructor"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } // Re-read the state.json from disk. diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index c2b8dcda2..4111ef66e 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -819,8 +819,30 @@ winrt::Windows::Foundation::IAsyncAction AppHost::_SaveWindowLayouts() if (_logic.ShouldUsePersistedLayout()) { - const auto layoutJsons = _windowManager.GetAllWindowLayouts(); - _logic.SaveWindowLayoutJsons(layoutJsons); + try + { + TraceLoggingWrite(g_hWindowsTerminalProvider, + "AppHost_SaveWindowLayouts_Collect", + TraceLoggingDescription("Logged when collecting window state"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + const auto layoutJsons = _windowManager.GetAllWindowLayouts(); + TraceLoggingWrite(g_hWindowsTerminalProvider, + "AppHost_SaveWindowLayouts_Save", + TraceLoggingDescription("Logged when writing window state"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + _logic.SaveWindowLayoutJsons(layoutJsons); + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + TraceLoggingWrite(g_hWindowsTerminalProvider, + "AppHost_SaveWindowLayouts_Failed", + TraceLoggingDescription("An error occured when collecting or writing window state"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + } } co_return; @@ -841,6 +863,12 @@ winrt::fire_and_forget AppHost::_SaveWindowLayoutsRepeat() // per 10 seconds, if a save is requested by another source simultaneously. if (_getWindowLayoutThrottler.has_value()) { + TraceLoggingWrite(g_hWindowsTerminalProvider, + "AppHost_requestGetLayout", + TraceLoggingDescription("Logged when triggering a throttled write of the window state"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + _getWindowLayoutThrottler.value()(); } } From aea37520b3cdb1c1752a6c8e0ff598991518ce28 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 30 Sep 2021 12:47:03 -0500 Subject: [PATCH 2/2] we want this --- src/cascadia/TerminalApp/AppLogic.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 4efbb57a2..350426f92 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -886,14 +886,19 @@ namespace winrt::TerminalApp::implementation // actual file you wrote. So listen for that too. wil::FolderChangeEvents::FileName | wil::FolderChangeEvents::LastWriteTime, [this, settingsBasename = settingsPath.filename()](wil::FolderChangeEvent, PCWSTR fileModified) { - static const auto appState{ ApplicationState::SharedInstance() }; + // DO NOT create a static reference to + // ApplicationState::SharedInstance here. See + // https://github.com/microsoft/terminal/pull/11222/files/9ff2775122a496fb8b1bcc7a0b83a64ce5b26c5f#r719627541 + // for why. ApplicationState::SharedInstance already caches it's + // own static ref. + const winrt::hstring modifiedBasename{ std::filesystem::path{ fileModified }.filename().c_str() }; if (modifiedBasename == settingsBasename) { _reloadSettings->Run(); } - else if (appState.IsStatePath(modifiedBasename)) + else if (ApplicationState::SharedInstance().IsStatePath(modifiedBasename)) { _reloadState(); }