Skip to content

Commit 414adf1

Browse files
miniksaDHowett
authored andcommitted
Switch FAIL_FAST to LOG for starting inbound connection server on monarch startup (#10261)
Stop startup crash by logging when monarch fails to register inbound connections, but still crash when COM attempted to start us ## References - See also #10243 ## PR Checklist * [x] Closes #10233 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA ## Detailed Description of the Pull Request / Additional comments - This should stop the crash on launch until we can get the internal teams to resolve the catalog issue - I left the COM -Embedding start fail fast though so it won't take forever to time out (as default timeout is 3-5 minutes). I will change that if it becomes necessary. ## Validation Steps Performed - I basically have to guess at this one based on the crash dump and Watson logs because it happens sporadically when the platform messes up on us. (cherry picked from commit d8647e0)
1 parent 3533aa2 commit 414adf1

File tree

3 files changed

+28
-6
lines changed

3 files changed

+28
-6
lines changed

src/cascadia/TerminalApp/AppLogic.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,7 @@ namespace winrt::TerminalApp::implementation
12061206
// in and be routed to an event with no handlers or a non-ready Page.
12071207
if (_appArgs.IsHandoffListener())
12081208
{
1209-
SetInboundListener();
1209+
_root->SetInboundListener(true);
12101210
}
12111211
}
12121212

@@ -1222,7 +1222,7 @@ namespace winrt::TerminalApp::implementation
12221222
// - <none>
12231223
void AppLogic::SetInboundListener()
12241224
{
1225-
_root->SetInboundListener();
1225+
_root->SetInboundListener(false);
12261226
}
12271227

12281228
// Method Description:

src/cascadia/TerminalApp/TerminalPage.cpp

+24-3
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,27 @@ namespace winrt::TerminalApp::implementation
356356
// we would be left with an empty terminal frame with no tabs.
357357
// Instead, crash out so COM sees the server die and things unwind
358358
// without a weird empty frame window.
359-
CATCH_FAIL_FAST()
359+
catch (...)
360+
{
361+
// However, we cannot always fail fast because of MSFT:33501832. Sometimes the COM catalog
362+
// tears the state between old and new versions and fails here for that reason.
363+
// As we're always becoming an inbound server in the monarch, even when COM didn't strictly
364+
// ask us yet...we might just crash always.
365+
// Instead... we're going to differentiate. If COM started us... we will fail fast
366+
// so it sees the process die and falls back.
367+
// If we were just starting normally as a Monarch and opportunistically listening for
368+
// inbound connections... then we'll just log the failure and move on assuming
369+
// the version state is torn and will fix itself whenever the packaging upgrade
370+
// tasks decide to clean up.
371+
if (_isEmbeddingInboundListener)
372+
{
373+
FAIL_FAST_CAUGHT_EXCEPTION();
374+
}
375+
else
376+
{
377+
LOG_CAUGHT_EXCEPTION();
378+
}
379+
}
360380
}
361381
}
362382

@@ -1981,12 +2001,13 @@ namespace winrt::TerminalApp::implementation
19812001
// listener for command-line tools attempting to join this Terminal
19822002
// through the default application channel.
19832003
// Arguments:
1984-
// - <none> - Implicitly sets to true. Default page state is false.
2004+
// - isEmbedding - True if COM started us to be a server. False if we're doing it of our own accord.
19852005
// Return Value:
19862006
// - <none>
1987-
void TerminalPage::SetInboundListener()
2007+
void TerminalPage::SetInboundListener(bool isEmbedding)
19882008
{
19892009
_shouldStartInboundListener = true;
2010+
_isEmbeddingInboundListener = isEmbedding;
19902011

19912012
// If the page has already passed the NotInitialized state,
19922013
// then it is ready-enough for us to just start this immediately.

src/cascadia/TerminalApp/TerminalPage.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ namespace winrt::TerminalApp::implementation
7979
bool AlwaysOnTop() const;
8080

8181
void SetStartupActions(std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
82-
void SetInboundListener();
82+
void SetInboundListener(bool isEmbedding);
8383
static std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> ConvertExecuteCommandlineToActions(const Microsoft::Terminal::Settings::Model::ExecuteCommandlineArgs& args);
8484

8585
winrt::TerminalApp::IDialogPresenter DialogPresenter() const;
@@ -175,6 +175,7 @@ namespace winrt::TerminalApp::implementation
175175

176176
Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::ActionAndArgs> _startupActions;
177177
bool _shouldStartInboundListener{ false };
178+
bool _isEmbeddingInboundListener{ false };
178179

179180
std::shared_ptr<Toast> _windowIdToast{ nullptr };
180181
std::shared_ptr<Toast> _windowRenameFailedToast{ nullptr };

0 commit comments

Comments
 (0)