Skip to content

Commit

Permalink
[DefApp] Move from Monarch multi instance servers to Peasant single i…
Browse files Browse the repository at this point in the history
…nstance servers (#10823)

- Monarch no longer sets itself up as a `CTerminalHandoff` multi instance server by default
- In fact, `CTerminalHandoff` will only ever be a single instance server 
- When COM needs a `CTerminalHandoff`, it launches `wt.exe -embedding`, which gets picked up by the Monarch and then gets handed off to itself/peasant depending on user settings.
- Peasant now recognizes the `-embedding` commandline and will start a `CTerminalHandoff` single instance listener, and receives the connection into a new tab.

Closes #10358
  • Loading branch information
leonMSFT authored Aug 5, 2021
1 parent 0b4839d commit 76793b1
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 50 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,12 @@ namespace winrt::TerminalApp::implementation
{
if (args == nullptr)
{
_OpenNewTab(nullptr);
LOG_IF_FAILED(_OpenNewTab(nullptr));
args.Handled(true);
}
else if (const auto& realArgs = args.ActionArgs().try_as<NewTabArgs>())
{
_OpenNewTab(realArgs.TerminalArgs());
LOG_IF_FAILED(_OpenNewTab(realArgs.TerminalArgs()));
args.Handled(true);
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,11 @@ namespace winrt::TerminalApp::implementation
auto actions = winrt::single_threaded_vector<ActionAndArgs>(std::move(appArgs.GetStartupActions()));

_root->ProcessStartupActions(actions, false, cwd);

if (appArgs.IsHandoffListener())
{
_root->SetInboundListener(true);
}
}
// Return the result of parsing with commandline, though it may or may not be used.
return result;
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace winrt::TerminalApp::implementation
// - existingConnection: An optional connection that is already established to a PTY
// for this tab to host instead of creating one.
// If not defined, the tab will create the connection.
void TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection)
HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection)
try
{
const auto profileGuid{ _settings.GetProfileForArgs(newTerminalArgs) };
Expand Down Expand Up @@ -89,8 +89,10 @@ namespace winrt::TerminalApp::implementation
TraceLoggingWideString(schemeName.data(), "SchemeName", "Color scheme set in the settings"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));

return S_OK;
}
CATCH_LOG();
CATCH_RETURN();

// Method Description:
// - Creates a new tab with the given settings. If the tab bar is not being
Expand Down
69 changes: 37 additions & 32 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "RenameWindowRequestedArgs.g.cpp"
#include "../inc/WindowingBehavior.h"

#include <til/latch.h>

using namespace winrt;
using namespace winrt::Windows::Foundation::Collections;
using namespace winrt::Windows::UI::Xaml;
Expand Down Expand Up @@ -348,34 +350,12 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::StartInboundListener();
}
// If we failed to start the listener, it will throw.
// We should fail fast here or the Terminal will be in a very strange state.
// We only start the listener if the Terminal was started with the COM server
// `-Embedding` flag and we make no tabs as a result.
// Therefore, if the listener cannot start itself up to make that tab with
// the inbound connection that caused the COM activation in the first place...
// we would be left with an empty terminal frame with no tabs.
// Instead, crash out so COM sees the server die and things unwind
// without a weird empty frame window.
// We don't want to fail fast here because if a peasant has some trouble with
// starting the listener, we don't want it to crash and take all its tabs down
// with it.
catch (...)
{
// However, we cannot always fail fast because of MSFT:33501832. Sometimes the COM catalog
// tears the state between old and new versions and fails here for that reason.
// As we're always becoming an inbound server in the monarch, even when COM didn't strictly
// ask us yet...we might just crash always.
// Instead... we're going to differentiate. If COM started us... we will fail fast
// so it sees the process die and falls back.
// If we were just starting normally as a Monarch and opportunistically listening for
// inbound connections... then we'll just log the failure and move on assuming
// the version state is torn and will fix itself whenever the packaging upgrade
// tasks decide to clean up.
if (_isEmbeddingInboundListener)
{
FAIL_FAST_CAUGHT_EXCEPTION();
}
else
{
LOG_CAUGHT_EXCEPTION();
}
LOG_CAUGHT_EXCEPTION();
}
}
}
Expand Down Expand Up @@ -759,7 +739,7 @@ namespace winrt::TerminalApp::implementation
}
else
{
this->_OpenNewTab(newTerminalArgs);
LOG_IF_FAILED(this->_OpenNewTab(newTerminalArgs));
}
}

Expand Down Expand Up @@ -2405,13 +2385,38 @@ namespace winrt::TerminalApp::implementation
return _isAlwaysOnTop;
}

void TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection)
HRESULT TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection)
{
// TODO: GH 9458 will give us more context so we can try to choose a better profile.
_OpenNewTab(nullptr, connection);
// We need to be on the UI thread in order for _OpenNewTab to run successfully.
// HasThreadAccess will return true if we're currently on a UI thread and false otherwise.
// When we're on a COM thread, we'll need to dispatch the calls to the UI thread
// and wait on it hence the locking mechanism.
if (Dispatcher().HasThreadAccess())
{
// TODO: GH 9458 will give us more context so we can try to choose a better profile.
auto hr = _OpenNewTab(nullptr, connection);

// Request a summon of this window to the foreground
_SummonWindowRequestedHandlers(*this, nullptr);
// Request a summon of this window to the foreground
_SummonWindowRequestedHandlers(*this, nullptr);

return hr;
}
else
{
til::latch latch{ 1 };
HRESULT finalVal = S_OK;

Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [&]() {
finalVal = _OpenNewTab(nullptr, connection);

_SummonWindowRequestedHandlers(*this, nullptr);

latch.count_down();
});

latch.wait();
return finalVal;
}
}

// Method Description:
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ namespace winrt::TerminalApp::implementation

void _CreateNewTabFlyout();
void _OpenNewTabDropdown();
void _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
void _CreateNewTabFromSettings(GUID profileGuid, const Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(GUID profileGuid, Microsoft::Terminal::Settings::Model::TerminalSettings settings);

Expand Down Expand Up @@ -344,7 +344,7 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::Settings::Model::Command _lastPreviewedCommand{ nullptr };
winrt::Microsoft::Terminal::Settings::Model::TerminalSettings _originalSettings{ nullptr };

void _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
HRESULT _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
void _HandleToggleInboundPty(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);

void _WindowRenamerActionClick(const IInspectable& sender, const IInspectable& eventArgs);
Expand Down
27 changes: 21 additions & 6 deletions src/cascadia/TerminalConnection/CTerminalHandoff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ using namespace Microsoft::WRL;
static NewHandoffFunction _pfnHandoff = nullptr;
// The registration ID of the class object for clean up later
static DWORD g_cTerminalHandoffRegistration = 0;
// Mutex so we only do start/stop/establish one at a time.
static std::shared_mutex _mtx;

// Routine Description:
// - Starts listening for TerminalHandoff requests by registering
Expand All @@ -19,9 +21,11 @@ static DWORD g_cTerminalHandoffRegistration = 0;
// - pfnHandoff - Function to callback when a handoff is received
// Return Value:
// - S_OK, E_NOT_VALID_STATE (start called when already started) or relevant COM registration error.
HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff) noexcept
HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff)
try
{
std::unique_lock lock{ _mtx };

RETURN_HR_IF(E_NOT_VALID_STATE, _pfnHandoff != nullptr);

const auto classFactory = Make<SimpleClassFactory<CTerminalHandoff>>();
Expand All @@ -31,7 +35,7 @@ try
ComPtr<IUnknown> unk;
RETURN_IF_FAILED(classFactory.As(&unk));

RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_MULTIPLEUSE, &g_cTerminalHandoffRegistration));
RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_SINGLEUSE, &g_cTerminalHandoffRegistration));

_pfnHandoff = pfnHandoff;

Expand All @@ -46,8 +50,10 @@ CATCH_RETURN()
// - <none>
// Return Value:
// - S_OK, E_NOT_VALID_STATE (stop called when not started), or relevant COM class revoke error
HRESULT CTerminalHandoff::s_StopListening() noexcept
HRESULT CTerminalHandoff::s_StopListening()
{
std::unique_lock lock{ _mtx };

RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);

_pfnHandoff = nullptr;
Expand Down Expand Up @@ -91,10 +97,19 @@ static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
// - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle`
// error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error
// from the registered handler event function.
HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept
HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client)
{
// Stash a local copy of _pfnHandoff before we stop listening.
auto localPfnHandoff = _pfnHandoff;

// Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
// COM does not automatically clean that up for us. We must do it.
s_StopListening();

std::unique_lock lock{ _mtx };

// Report an error if no one registered a handoff function before calling this.
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);

// Duplicate the handles from what we received.
// The contract with COM specifies that any HANDLEs we receive from the caller belong
Expand All @@ -108,5 +123,5 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign
RETURN_IF_FAILED(_duplicateHandle(client, client));

// Call registered handler from when we started listening.
return _pfnHandoff(in, out, signal, ref, server, client);
return localPfnHandoff(in, out, signal, ref, server, client);
}
6 changes: 3 additions & 3 deletions src/cascadia/TerminalConnection/CTerminalHandoff.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff))
HANDLE signal,
HANDLE ref,
HANDLE server,
HANDLE client) noexcept override;
HANDLE client) override;

#pragma endregion

static HRESULT s_StartListening(NewHandoffFunction pfnHandoff) noexcept;
static HRESULT s_StopListening() noexcept;
static HRESULT s_StartListening(NewHandoffFunction pfnHandoff);
static HRESULT s_StopListening();
};

// Disable warnings from the CoCreatableClass macro as the value it provides for
Expand Down
3 changes: 0 additions & 3 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,9 +656,6 @@ void AppHost::_BecomeMonarch(const winrt::Windows::Foundation::IInspectable& /*s
const winrt::Windows::Foundation::IInspectable& /*args*/)
{
_setupGlobalHotkeys();

// The monarch is just going to be THE listener for inbound connections.
_listenForInboundConnections();
}

void AppHost::_listenForInboundConnections()
Expand Down

0 comments on commit 76793b1

Please sign in to comment.