Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Defapp] Use real HPCON for PTY management; Have Monarch always listen for connections #10170

Merged
12 commits merged into from
May 24, 2021
Merged
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/alphabet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ BBBBBBBBBBBBBBDDDD
BBBBBCCC
BBBBCCCCC
BBGGRR
CCE
EFG
EFGh
QQQQQQQQQQABCDEFGHIJ
Expand Down
1 change: 0 additions & 1 deletion .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1849,7 +1849,6 @@ psp
PSPCB
psr
PSTR
psuedoconsole
psz
ptch
ptr
Expand Down
2 changes: 1 addition & 1 deletion samples/ConPTY/GUIConsole/GUIConsole.ConPTY/Terminal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public Terminal()
}

/// <summary>
/// Start the psuedoconsole and run the process as shown in
/// Start the pseudoconsole and run the process as shown in
/// https://docs.microsoft.com/en-us/windows/console/creating-a-pseudoconsole-session#creating-the-pseudoconsole
/// </summary>
/// <param name="command">the command to run, e.g. cmd.exe</param>
Expand Down
2 changes: 1 addition & 1 deletion samples/ConPTY/MiniTerm/MiniTerm/Terminal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private static void EnableVirtualTerminalSequenceProcessing()
}

/// <summary>
/// Start the psuedoconsole and run the process as shown in
/// Start the pseudoconsole and run the process as shown in
/// https://docs.microsoft.com/en-us/windows/console/creating-a-pseudoconsole-session#creating-the-pseudoconsole
/// </summary>
/// <param name="command">the command to run, e.g. cmd.exe</param>
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/CascadiaPackage/Package-Dev.appxmanifest
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
<com:ComInterface>
<com:ProxyStub Id="DEC4804D-56D1-4F73-9FBE-6828E7C85C56" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
<com:Interface Id="2B607BC1-43EB-40C3-95AE-2856ADDB7F23" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/>
<com:Interface Id="FA1E3AB4-9AEC-4A3C-96CA-E6078C30BD74" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/>
<com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/>
</com:ComInterface>
</com:Extension>
<com:Extension Category="windows.comServer">
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/CascadiaPackage/Package-Pre.appxmanifest
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
<com:ComInterface>
<com:ProxyStub Id="1833E661-CC81-4DD0-87C6-C2F74BD39EFA" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
<com:Interface Id="2B607BC1-43EB-40C3-95AE-2856ADDB7F23" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/>
<com:Interface Id="FA1E3AB4-9AEC-4A3C-96CA-E6078C30BD74" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/>
<com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/>
</com:ComInterface>
</com:Extension>
<com:Extension Category="windows.comServer">
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/CascadiaPackage/Package.appxmanifest
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
<com:ComInterface>
<com:ProxyStub Id="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
<com:Interface Id="2B607BC1-43EB-40C3-95AE-2856ADDB7F23" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/>
<com:Interface Id="FA1E3AB4-9AEC-4A3C-96CA-E6078C30BD74" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/>
<com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/>
</com:ComInterface>
</com:Extension> -->
<com:Extension Category="windows.comServer">
Expand Down
14 changes: 13 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1206,13 +1206,25 @@ namespace winrt::TerminalApp::implementation
// in and be routed to an event with no handlers or a non-ready Page.
if (_appArgs.IsHandoffListener())
{
_root->SetInboundListener();
SetInboundListener();
}
}

return result;
}

// Method Description:
// - Triggers the setup of the listener for incoming console connections
// from the operating system.
// Arguments:
// - <none>
// Return Value:
// - <none>
void AppLogic::SetInboundListener()
{
_root->SetInboundListener();
}

// Method Description:
// - Parse the provided commandline arguments into actions, and try to
// perform them immediately.
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ namespace winrt::TerminalApp::implementation

Windows::UI::Xaml::UIElement GetRoot() noexcept;

void SetInboundListener();

hstring Title();
void TitlebarClicked();
bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ namespace TerminalApp
void LoadSettings();
Windows.UI.Xaml.UIElement GetRoot();

void SetInboundListener();

String Title { get; };

Boolean FocusMode { get; };
Expand Down
52 changes: 37 additions & 15 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,23 +325,38 @@ namespace winrt::TerminalApp::implementation
// This MUST be done after we've registered the event listener for the new connections
// or the COM server might start receiving requests on another thread and dispatch
// them to nowhere.
if (_shouldStartInboundListener)
_StartInboundListener();
}
}

// Routine Description:
// - Will start the listener for inbound console handoffs if we have already determined
// that we should do so.
// NOTE: Must be after TerminalPage::_OnNewConnection has been connected up.
// Arguments:
// - <unused> - Looks at _shouldStartInboundListener
// Return Value:
// - <none> - May fail fast if setup fails as that would leave us in a weird state.
void TerminalPage::_StartInboundListener()
{
if (_shouldStartInboundListener)
{
_shouldStartInboundListener = false;

try
{
try
{
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.
CATCH_FAIL_FAST()
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.
CATCH_FAIL_FAST()
}
}

Expand Down Expand Up @@ -1989,6 +2004,13 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::SetInboundListener()
{
_shouldStartInboundListener = true;

// If the page has already passed the NotInitialized state,
// then it is ready-enough for us to just start this immediately.
if (_startupState != StartupState::NotInitialized)
{
_StartInboundListener();
}
}

winrt::TerminalApp::IDialogPresenter TerminalPage::DialogPresenter() const
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ namespace winrt::TerminalApp::implementation
void _SetNewTabButtonColor(const Windows::UI::Color& color, const Windows::UI::Color& accentColor);
void _ClearNewTabButtonColor();

void _StartInboundListener();

void _CompleteInitialization();

void _FocusActiveControl(IInspectable sender, IInspectable eventArgs);
Expand Down
12 changes: 8 additions & 4 deletions src/cascadia/TerminalConnection/CTerminalHandoff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
// - in - PTY input handle that we will read from
// - out - PTY output handle that we will write to
// - signal - PTY signal handle for out of band messaging
// - process - Process handle to client so we can track its lifetime and exit appropriately
// - ref - Client reference handle for console session so it stays alive until we let go
// - server - PTY process handle to track for lifetime/cleanup
// - client - Process handle to client so we can track its lifetime and exit appropriately
// Return Value:
// - 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 process) noexcept
HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept
{
// Report an error if no one registered a handoff function before calling this.
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
Expand All @@ -101,8 +103,10 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign
RETURN_IF_FAILED(_duplicateHandle(in, in));
RETURN_IF_FAILED(_duplicateHandle(out, out));
RETURN_IF_FAILED(_duplicateHandle(signal, signal));
RETURN_IF_FAILED(_duplicateHandle(process, process));
RETURN_IF_FAILED(_duplicateHandle(ref, ref));
RETURN_IF_FAILED(_duplicateHandle(server, server));
RETURN_IF_FAILED(_duplicateHandle(client, client));
miniksa marked this conversation as resolved.
Show resolved Hide resolved

// Call registered handler from when we started listening.
return _pfnHandoff(in, out, signal, process);
return _pfnHandoff(in, out, signal, ref, server, client);
}
6 changes: 4 additions & 2 deletions src/cascadia/TerminalConnection/CTerminalHandoff.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Author(s):
#define __CLSID_CTerminalHandoff "051F34EE-C1FD-4B19-AF75-9BA54648434C"
#endif

using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE);
using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, HANDLE);

struct __declspec(uuid(__CLSID_CTerminalHandoff))
CTerminalHandoff : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::RuntimeClassType::ClassicCom>, ITerminalHandoff>
Expand All @@ -35,7 +35,9 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff))
STDMETHODIMP EstablishPtyHandoff(HANDLE in,
HANDLE out,
HANDLE signal,
HANDLE process) noexcept override;
HANDLE ref,
HANDLE server,
HANDLE client) noexcept override;

#pragma endregion

Expand Down
8 changes: 5 additions & 3 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
ConptyConnection::ConptyConnection(const HANDLE hSig,
const HANDLE hIn,
const HANDLE hOut,
const HANDLE hRef,
const HANDLE hServerProcess,
const HANDLE hClientProcess) :
_initialRows{ 25 },
_initialCols{ 80 },
Expand All @@ -208,7 +210,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_inPipe{ hIn },
_outPipe{ hOut }
{
hSig; // TODO: GH 9464 this needs to be packed into the hpcon
THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC));
if (_guid == guid{})
{
_guid = Utils::CreateGuid();
Expand Down Expand Up @@ -500,10 +502,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
winrt::event_token ConptyConnection::NewConnection(NewConnectionHandler const& handler) { return _newConnectionHandlers.add(handler); };
void ConptyConnection::NewConnection(winrt::event_token const& token) { _newConnectionHandlers.remove(token); };

HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE process) noexcept
HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept
try
{
auto conn = winrt::make<implementation::ConptyConnection>(signal, in, out, process);
auto conn = winrt::make<implementation::ConptyConnection>(signal, in, out, ref, server, client);
_newConnectionHandlers(conn);

return S_OK;
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
ConptyConnection(const HANDLE hSig,
const HANDLE hIn,
const HANDLE hOut,
const HANDLE hRef,
const HANDLE hServerProcess,
const HANDLE hClientProcess);

ConptyConnection(
Expand Down Expand Up @@ -54,7 +56,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
void _indicateExitWithStatus(unsigned int status) noexcept;
void _ClientTerminated() noexcept;

static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE process) noexcept;
static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept;

uint32_t _initialRows{};
uint32_t _initialCols{};
Expand Down
5 changes: 4 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1664,7 +1664,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TermControl::_CursorPositionChanged(const IInspectable& /*sender*/,
const IInspectable& /*args*/)
{
_tsfTryRedrawCanvas->Run();
if (_tsfTryRedrawCanvas)
miniksa marked this conversation as resolved.
Show resolved Hide resolved
{
_tsfTryRedrawCanvas->Run();
}
}

hstring TermControl::Title()
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Launch.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
<!-- profile name and author -->
<ColumnDefinition Width="*" />
<!-- version -->
<ColumnDefinition Width="48" />
<ColumnDefinition Width="Auto" />
</Grid.ColumnDefinitions>

<Grid.RowDefinitions>
Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,14 @@ 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()
{
_logic.SetInboundListener();
}

winrt::fire_and_forget AppHost::_setupGlobalHotkeys()
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class AppHost

bool _LazyLoadDesktopManager();

void _listenForInboundConnections();
winrt::fire_and_forget _setupGlobalHotkeys();
winrt::fire_and_forget _createNewTerminalWindow(winrt::Microsoft::Terminal::Settings::Model::GlobalSummonArgs args);
void _HandleSettingsChanged(const winrt::Windows::Foundation::IInspectable& sender,
Expand Down
4 changes: 3 additions & 1 deletion src/host/proxy/ITerminalHandoff.idl
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import "ocidl.idl";

[
object,
uuid(FA1E3AB4-9AEC-4A3C-96CA-E6078C30BD74)
uuid(59D55CCE-FC8A-48B4-ACE8-0A9286C6557F)
] interface ITerminalHandoff : IUnknown
{
HRESULT EstablishPtyHandoff([in, system_handle(sh_pipe)] HANDLE in,
[in, system_handle(sh_pipe)] HANDLE out,
[in, system_handle(sh_pipe)] HANDLE signal,
[in, system_handle(sh_file)] HANDLE ref,
[in, system_handle(sh_process)] HANDLE server,
[in, system_handle(sh_process)] HANDLE client);
};
11 changes: 11 additions & 0 deletions src/host/srvinit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "../types/inc/GlyphWidth.hpp"

#include "../server/DeviceHandle.h"
#include "../server/Entrypoints.h"
#include "../server/IoSorter.h"

Expand Down Expand Up @@ -414,13 +415,23 @@ try
wil::unique_handle clientProcess{ OpenProcess(PROCESS_QUERY_INFORMATION | SYNCHRONIZE, TRUE, static_cast<DWORD>(connectMessage->Descriptor.Process)) };
RETURN_LAST_ERROR_IF_NULL(clientProcess.get());

wil::unique_handle refHandle;
RETURN_IF_NTSTATUS_FAILED(DeviceHandle::CreateClientHandle(refHandle.addressof(),
Server,
L"\\Reference",
FALSE));

const auto serverProcess = GetCurrentProcess();

::Microsoft::WRL::ComPtr<ITerminalHandoff> handoff;

RETURN_IF_FAILED(CoCreateInstance(g.handoffTerminalClsid.value(), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff)));

RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(),
outPipeTheirSide.get(),
signalPipeTheirSide.get(),
refHandle.get(),
serverProcess,
clientProcess.get()));

inPipeTheirSide.release();
Expand Down
2 changes: 2 additions & 0 deletions src/inc/conpty-static.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ HRESULT WINAPI ConptyResizePseudoConsole(HPCON hPC, COORD size);

VOID WINAPI ConptyClosePseudoConsole(HPCON hPC);

HRESULT WINAPI ConptyPackPseudoConsole(HANDLE hServerProcess, HANDLE hRef, HANDLE hSignal, HPCON* phPC);

#ifdef __cplusplus
}
#endif
Loading