From 446f2807573ffda411f548a519835d15cacdcd9b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 4 Apr 2022 17:35:02 -0500 Subject: [PATCH] Try to silently fall back to a local monarch (#12825) This is a crazy idea Dustin and I had. > we can't repro this at will. But we kinda have an idea of where the deref is. We don't know if the small patch (throw, and try again) will fix it. We're sure that the "just fall back to an isolated monarch" will work. I'd almost rather take a build testing the small patch first, to see if that works > This might seem crazy > in 1.12, isolated monarch. In 1.13, "small patch". In 1.14, we can wait and see I can write more details in the morning. It's 5pm here so if we want this today, here it is. @dhowett double check my velocity flag logic here. Should be always true for Release, and off for Dev, Preview. * [x] closes #12774 --- src/cascadia/Remoting/Monarch.h | 2 +- src/cascadia/Remoting/WindowManager.cpp | 46 +++++++++++++++++++++++++ src/features.xml | 11 ++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/cascadia/Remoting/Monarch.h b/src/cascadia/Remoting/Monarch.h index ae85396b29a..a9eac206daf 100644 --- a/src/cascadia/Remoting/Monarch.h +++ b/src/cascadia/Remoting/Monarch.h @@ -27,7 +27,7 @@ constexpr GUID Monarch_clsid 0x7eb1, 0x4f3e, { - 0x85, 0xf5, 0x8b, 0xdd, 0x73, 0x86, 0xcc, 0xe3 + 0x85, 0xf5, 0x8b, 0xdd, 0x73, 0x86, 0xcc, 0xe4 } }; diff --git a/src/cascadia/Remoting/WindowManager.cpp b/src/cascadia/Remoting/WindowManager.cpp index 89f337830bf..d9d42100c8e 100644 --- a/src/cascadia/Remoting/WindowManager.cpp +++ b/src/cascadia/Remoting/WindowManager.cpp @@ -347,6 +347,52 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation void WindowManager::_createMonarchAndCallbacks() { _createMonarch(); + + if (_monarch == nullptr) + { + // See MSFT:38540483, GH#12774 for details. + TraceLoggingWrite(g_hRemotingProvider, + "WindowManager_NullMonarchTryAgain", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // Here we're gonna just give it a quick second try.Probably not + // definitive, but might help. + _createMonarch(); + } + + if (_monarch == nullptr) + { + // See MSFT:38540483, GH#12774 for details. + if constexpr (Feature_IsolatedMonarchMode::IsEnabled()) + { + // Fall back to having a in proc monarch. Were now isolated from + // other windows. This is a pretty torn state, but at least we + // didn't just explode. + TraceLoggingWrite(g_hRemotingProvider, + "WindowManager_NullMonarchIsolateMode", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + _monarch = winrt::make(); + } + else + { + // The monarch is null. We're hoping that we can find another, + // hopefully us. We're gonna go back around the loop again and + // see what happens. If this is really an infinite loop (where + // the OS won't even give us back US as the monarch), then I + // suppose we'll find out soon enough. + TraceLoggingWrite(g_hRemotingProvider, + "WindowManager_NullMonarchTryAgain", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + winrt::hresult_error(E_UNEXPECTED, L"Did not expect the Monarch to ever be null"); + } + } + + // We're pretty confident that we have a Monarch here. + // Save the result of checking if we're the king. We want to avoid // unnecessary calls back and forth if we can. _isKing = _areWeTheKing(); diff --git a/src/features.xml b/src/features.xml index c2fde9d2765..d08b97a9026 100644 --- a/src/features.xml +++ b/src/features.xml @@ -95,4 +95,15 @@ Preview + + + Feature_IsolatedMonarchMode + Enables a test flag for MSFT:38540483. When enabled, if we ever create a null Monarch, we'll stealthily try to fall back to an in-proc monarch instance. + AlwaysEnabled + + + Dev + Preview + +