diff --git a/src/cascadia/CascadiaPackage/Package-Can.appxmanifest b/src/cascadia/CascadiaPackage/Package-Can.appxmanifest
index 72b0a64878..e8ff744104 100644
--- a/src/cascadia/CascadiaPackage/Package-Can.appxmanifest
+++ b/src/cascadia/CascadiaPackage/Package-Can.appxmanifest
@@ -111,8 +111,7 @@
-
-
+
diff --git a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest
index d8ceefaeef..9f68b6f3e4 100644
--- a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest
+++ b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest
@@ -111,8 +111,7 @@
-
-
+
diff --git a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest
index 3a7bf26e0f..665b5b7dde 100644
--- a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest
+++ b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest
@@ -200,8 +200,7 @@
-
-
+
diff --git a/src/cascadia/CascadiaPackage/Package.appxmanifest b/src/cascadia/CascadiaPackage/Package.appxmanifest
index f04863da27..c7ed197075 100644
--- a/src/cascadia/CascadiaPackage/Package.appxmanifest
+++ b/src/cascadia/CascadiaPackage/Package.appxmanifest
@@ -200,8 +200,7 @@
-
-
+
diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp
index cf33c4002a..a6edb23243 100644
--- a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp
+++ b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp
@@ -72,20 +72,6 @@ HRESULT CTerminalHandoff::s_StopListeningLocked()
return S_OK;
}
-// Routine Description:
-// - Helper to duplicate a handle to ourselves so we can keep holding onto it
-// after the caller frees the original one.
-// Arguments:
-// - in - Handle to duplicate
-// - out - Where to place the duplicated value
-// Return Value:
-// - S_OK or Win32 error from `::DuplicateHandle`
-static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
-{
- RETURN_IF_WIN32_BOOL_FALSE(::DuplicateHandle(GetCurrentProcess(), in, GetCurrentProcess(), &out, 0, FALSE, DUPLICATE_SAME_ACCESS));
- return S_OK;
-}
-
// Routine Description:
// - Receives the terminal handoff via COM from the other process,
// duplicates handles as COM will free those given on the way out,
@@ -102,7 +88,7 @@ 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, TERMINAL_STARTUP_INFO startupInfo)
+HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo)
{
try
{
@@ -120,19 +106,8 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign
// Report an error if no one registered a handoff function before calling this.
THROW_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
- // to the caller and will be freed when we leave the scope of this method.
- // Making our own duplicate copy ensures they hang around in our lifetime.
- THROW_IF_FAILED(_duplicateHandle(in, in));
- THROW_IF_FAILED(_duplicateHandle(out, out));
- THROW_IF_FAILED(_duplicateHandle(signal, signal));
- THROW_IF_FAILED(_duplicateHandle(ref, ref));
- THROW_IF_FAILED(_duplicateHandle(server, server));
- THROW_IF_FAILED(_duplicateHandle(client, client));
-
// Call registered handler from when we started listening.
- THROW_IF_FAILED(localPfnHandoff(in, out, signal, ref, server, client, startupInfo));
+ THROW_IF_FAILED(localPfnHandoff(in, out, signal, reference, server, client, startupInfo));
#pragma warning(suppress : 26477)
TraceLoggingWrite(
diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.h b/src/cascadia/TerminalConnection/CTerminalHandoff.h
index 3d6c3d1987..440a2636f2 100644
--- a/src/cascadia/TerminalConnection/CTerminalHandoff.h
+++ b/src/cascadia/TerminalConnection/CTerminalHandoff.h
@@ -28,19 +28,13 @@ Author(s):
#define __CLSID_CTerminalHandoff "051F34EE-C1FD-4B19-AF75-9BA54648434C"
#endif
-using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, TERMINAL_STARTUP_INFO);
+using NewHandoffFunction = HRESULT (*)(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo);
struct __declspec(uuid(__CLSID_CTerminalHandoff))
- CTerminalHandoff : public Microsoft::WRL::RuntimeClass, ITerminalHandoff2>
+ CTerminalHandoff : public Microsoft::WRL::RuntimeClass, ITerminalHandoff3>
{
#pragma region ITerminalHandoff
- STDMETHODIMP EstablishPtyHandoff(HANDLE in,
- HANDLE out,
- HANDLE signal,
- HANDLE ref,
- HANDLE server,
- HANDLE client,
- TERMINAL_STARTUP_INFO startupInfo) override;
+ STDMETHODIMP EstablishPtyHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) override;
#pragma endregion
diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp
index c70a85e1dc..e33ddc7b0a 100644
--- a/src/cascadia/TerminalConnection/ConptyConnection.cpp
+++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp
@@ -5,7 +5,6 @@
#include "ConptyConnection.h"
#include
-#include
#include "CTerminalHandoff.h"
#include "LibraryResources.h"
@@ -173,33 +172,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}
CATCH_RETURN();
- ConptyConnection::ConptyConnection(const HANDLE hSig,
- const HANDLE hIn,
- const HANDLE hOut,
- const HANDLE hRef,
- const HANDLE hServerProcess,
- const HANDLE hClientProcess,
- const TERMINAL_STARTUP_INFO& startupInfo) :
- _rows{ 25 },
- _cols{ 80 },
- _inPipe{ hIn },
- _outPipe{ hOut }
+ // Who decided that?
+#pragma warning(suppress : 26455) // Default constructor should not throw. Declare it 'noexcept' (f.6).
+ ConptyConnection::ConptyConnection()
{
- _sessionId = Utils::CreateGuid();
-
- THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC));
- _piClient.hProcess = hClientProcess;
-
- _startupInfo.title = winrt::hstring{ startupInfo.pszTitle, SysStringLen(startupInfo.pszTitle) };
- _startupInfo.iconPath = winrt::hstring{ startupInfo.pszIconPath, SysStringLen(startupInfo.pszIconPath) };
- _startupInfo.iconIndex = startupInfo.iconIndex;
- _startupInfo.showWindow = startupInfo.wShowWindow;
-
- try
- {
- _commandline = _commandlineFromProcess(hClientProcess);
- }
- CATCH_LOG()
}
// Function Description:
@@ -319,6 +295,53 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}
}
+ static wil::unique_hfile duplicateHandle(const HANDLE in)
+ {
+ wil::unique_hfile h;
+ THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(), in, GetCurrentProcess(), h.addressof(), 0, FALSE, DUPLICATE_SAME_ACCESS));
+ return h;
+ }
+
+ // Misdiagnosis: out is being tested right in the first line.
+#pragma warning(suppress : 26430) // Symbol 'out' is not tested for nullness on all paths (f.23).
+ void ConptyConnection::InitializeFromHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo)
+ {
+ THROW_HR_IF(E_UNEXPECTED, !in || !out || !startupInfo);
+
+ _sessionId = Utils::CreateGuid();
+
+ wil::unique_hfile inPipePseudoConsoleSide;
+ wil::unique_hfile outPipePseudoConsoleSide;
+ THROW_IF_WIN32_BOOL_FALSE(CreatePipe(&inPipePseudoConsoleSide, &_inPipe, nullptr, 0));
+ THROW_IF_WIN32_BOOL_FALSE(CreatePipe(&_outPipe, &outPipePseudoConsoleSide, nullptr, 0));
+
+ auto ownedSignal = duplicateHandle(signal);
+ auto ownedReference = duplicateHandle(reference);
+ auto ownedServer = duplicateHandle(server);
+ auto ownedClient = duplicateHandle(client);
+
+ THROW_IF_FAILED(ConptyPackPseudoConsole(ownedServer.get(), ownedReference.get(), ownedSignal.get(), &_hPC));
+ ownedServer.release();
+ ownedReference.release();
+ ownedSignal.release();
+
+ _piClient.hProcess = ownedClient.release();
+
+ _startupInfo.title = winrt::hstring{ startupInfo->pszTitle, SysStringLen(startupInfo->pszTitle) };
+ _startupInfo.iconPath = winrt::hstring{ startupInfo->pszIconPath, SysStringLen(startupInfo->pszIconPath) };
+ _startupInfo.iconIndex = startupInfo->iconIndex;
+ _startupInfo.showWindow = startupInfo->wShowWindow;
+
+ try
+ {
+ _commandline = _commandlineFromProcess(_piClient.hProcess);
+ }
+ CATCH_LOG()
+
+ *in = inPipePseudoConsoleSide.release();
+ *out = outPipePseudoConsoleSide.release();
+ }
+
winrt::hstring ConptyConnection::Commandline() const
{
return _commandline;
@@ -618,12 +641,20 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
// won't wait for us, and the known exit points _do_.
auto strongThis{ get_strong() };
+ const auto cleanup = wil::scope_exit([this]() noexcept {
+ _LastConPtyClientDisconnected();
+ });
+
+ char buffer[128 * 1024];
+ DWORD read = 0;
+
+ til::u8state u8State;
+ std::wstring wstr;
+
// process the data of the output pipe in a loop
while (true)
{
- DWORD read{};
-
- const auto readFail{ !ReadFile(_outPipe.get(), _buffer.data(), gsl::narrow_cast(_buffer.size()), &read, nullptr) };
+ const auto readFail{ !ReadFile(_outPipe.get(), &buffer[0], sizeof(buffer), &read, nullptr) };
// When we call CancelSynchronousIo() in Close() this is the branch that's taken and gets us out of here.
if (_isStateAtOrBeyond(ConnectionState::Closing))
@@ -648,7 +679,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}
}
- const auto result{ til::u8u16(std::string_view{ _buffer.data(), read }, _u16Str, _u8State) };
+ const auto result{ til::u8u16(std::string_view{ &buffer[0], read }, wstr, u8State) };
if (FAILED(result))
{
// EXIT POINT
@@ -657,7 +688,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return gsl::narrow_cast(result);
}
- if (_u16Str.empty())
+ if (wstr.empty())
{
return 0;
}
@@ -679,7 +710,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}
// Pass the output to our registered event handlers
- TerminalOutput.raise(_u16Str);
+ TerminalOutput.raise(wstr);
}
return 0;
@@ -695,11 +726,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
::ConptyClosePseudoConsoleTimeout(hPC, 0);
}
- HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept
+ HRESULT ConptyConnection::NewHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) noexcept
try
{
- _newConnectionHandlers(winrt::make(signal, in, out, ref, server, client, startupInfo));
-
+ auto conn = winrt::make_self();
+ conn->InitializeFromHandoff(in, out, signal, reference, server, client, startupInfo);
+ _newConnectionHandlers(*std::move(conn));
return S_OK;
}
CATCH_RETURN()
diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h
index 1ce3b72ba9..5c604ec29a 100644
--- a/src/cascadia/TerminalConnection/ConptyConnection.h
+++ b/src/cascadia/TerminalConnection/ConptyConnection.h
@@ -13,16 +13,9 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
struct ConptyConnection : ConptyConnectionT, BaseTerminalConnection
{
- ConptyConnection(const HANDLE hSig,
- const HANDLE hIn,
- const HANDLE hOut,
- const HANDLE hRef,
- const HANDLE hServerProcess,
- const HANDLE hClientProcess,
- const TERMINAL_STARTUP_INFO& startupInfo);
-
- ConptyConnection() noexcept = default;
+ explicit ConptyConnection();
void Initialize(const Windows::Foundation::Collections::ValueSet& settings);
+ void InitializeFromHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo);
static winrt::fire_and_forget final_release(std::unique_ptr connection);
@@ -61,15 +54,15 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
private:
static void closePseudoConsoleAsync(HPCON hPC) noexcept;
- static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept;
+ static HRESULT NewHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) noexcept;
static winrt::hstring _commandlineFromProcess(HANDLE process);
HRESULT _LaunchAttachedClient() noexcept;
void _indicateExitWithStatus(unsigned int status) noexcept;
void _LastConPtyClientDisconnected() noexcept;
- til::CoordType _rows{};
- til::CoordType _cols{};
+ til::CoordType _rows = 120;
+ til::CoordType _cols = 30;
uint64_t _initialParentHwnd{ 0 };
hstring _commandline{};
hstring _startingDirectory{};
@@ -87,9 +80,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
wil::unique_process_information _piClient;
wil::unique_any _hPC;
- til::u8state _u8State{};
- std::wstring _u16Str{};
- std::array _buffer{};
DWORD _flags{ 0 };
til::env _initialEnv{};
diff --git a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
index 8fc12f493d..ed4a1ae94e 100644
--- a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
+++ b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
@@ -2586,7 +2586,6 @@ namespace RemotingUnitTests
try
{
const auto result = m1->ProposeCommandline(args);
- VERIFY_IS_FALSE(true, L"This should have thrown");
}
catch (const winrt::hresult_error& e)
{
@@ -2596,9 +2595,10 @@ namespace RemotingUnitTests
// This is the same check in WindowManager::_proposeToMonarch.
VERIFY_IS_TRUE(e.code() == RPC_SERVER_UNAVAILABLE_HR || e.code() == RPC_CALL_FAILED_HR);
+ return;
}
- // just don't catch other types of exceptions. They'll take out
- // TAEF, which will count as a failure.
+
+ VERIFY_FAIL(L"This should have thrown");
}
}
diff --git a/src/host/proxy/IConsoleHandoff.idl b/src/host/proxy/IConsoleHandoff.idl
index 3a07dd8469..f81999986e 100644
--- a/src/host/proxy/IConsoleHandoff.idl
+++ b/src/host/proxy/IConsoleHandoff.idl
@@ -1,8 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
-import "oaidl.idl";
-import "ocidl.idl";
+import "unknwn.idl";
typedef struct _CONSOLE_PORTABLE_ATTACH_MSG
{
diff --git a/src/host/proxy/ITerminalHandoff.idl b/src/host/proxy/ITerminalHandoff.idl
index 37906770f0..f48bc10f69 100644
--- a/src/host/proxy/ITerminalHandoff.idl
+++ b/src/host/proxy/ITerminalHandoff.idl
@@ -1,9 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
-import "oaidl.idl";
-import "ocidl.idl";
-
+import "unknwn.idl";
typedef struct _TERMINAL_STARTUP_INFO
{
@@ -33,6 +31,7 @@ typedef struct _TERMINAL_STARTUP_INFO
// versions of interfaces in the file here, even if the old version is no longer
// in use.
+// This was the original prototype. The reasons for changes to it are explained below.
[
object,
uuid(59D55CCE-FC8A-48B4-ACE8-0A9286C6557F)
@@ -47,6 +46,9 @@ typedef struct _TERMINAL_STARTUP_INFO
[in, system_handle(sh_process)] HANDLE client);
};
+// We didn't consider the need for TERMINAL_STARTUP_INFO, because Windows Terminal never had support for launching
+// .lnk files in the first place. They aren't executables after all, but rather a shell thing.
+// Its need quickly became apparent right after releasing ITerminalHandoff, which is why it was very short-lived.
[
object,
uuid(AA6B364F-4A50-4176-9002-0AE755E7B5EF)
@@ -60,3 +62,21 @@ typedef struct _TERMINAL_STARTUP_INFO
[in, system_handle(sh_process)] HANDLE client,
[in] TERMINAL_STARTUP_INFO startupInfo);
};
+
+// Quite a while later, we realized that passing the in/out handles as [in] instead of [out] has always been flawed.
+// It prevents the terminal from making choices over the pipe buffer size and whether to use overlapped IO or not.
+// The other handles remain [in] parameters because they have always been created internally by ConPTY.
+// Additionally, passing TERMINAL_STARTUP_INFO by-value was unusual under COM as structs are usually given by-ref.
+[
+ object,
+ uuid(6F23DA90-15C5-4203-9DB0-64E73F1B1B00)
+] interface ITerminalHandoff3 : IUnknown
+{
+ HRESULT EstablishPtyHandoff([out, system_handle(sh_pipe)] HANDLE* in,
+ [out, system_handle(sh_pipe)] HANDLE* out,
+ [in, system_handle(sh_pipe)] HANDLE signal,
+ [in, system_handle(sh_file)] HANDLE reference,
+ [in, system_handle(sh_process)] HANDLE server,
+ [in, system_handle(sh_process)] HANDLE client,
+ [in] const TERMINAL_STARTUP_INFO* startupInfo);
+};
diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp
index a29c31ad6f..8be3b28f48 100644
--- a/src/host/srvinit.cpp
+++ b/src/host/srvinit.cpp
@@ -457,19 +457,8 @@ try
wil::unique_handle signalPipeTheirSide;
wil::unique_handle signalPipeOurSide;
-
- wil::unique_handle inPipeTheirSide;
- wil::unique_handle inPipeOurSide;
-
- wil::unique_handle outPipeTheirSide;
- wil::unique_handle outPipeOurSide;
-
RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(signalPipeOurSide.addressof(), signalPipeTheirSide.addressof(), nullptr, 0));
- RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(inPipeOurSide.addressof(), inPipeTheirSide.addressof(), nullptr, 0));
-
- RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(outPipeTheirSide.addressof(), outPipeOurSide.addressof(), nullptr, 0));
-
TraceLoggingWrite(g_hConhostV2EventTraceProvider,
"SrvInit_ReceiveHandoff_OpenedPipes",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
@@ -491,7 +480,7 @@ try
const auto serverProcess = GetCurrentProcess();
- ::Microsoft::WRL::ComPtr handoff;
+ ::Microsoft::WRL::ComPtr handoff;
TraceLoggingWrite(g_hConhostV2EventTraceProvider,
"SrvInit_PrepareToCreateDelegationTerminal",
@@ -566,21 +555,21 @@ try
myStartupInfo.wShowWindow = settings.GetShowWindow();
- RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(),
- outPipeTheirSide.get(),
+ wil::unique_handle inPipeOurSide;
+ wil::unique_handle outPipeOurSide;
+ RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeOurSide.addressof(),
+ outPipeOurSide.addressof(),
signalPipeTheirSide.get(),
refHandle.get(),
serverProcess,
clientProcess.get(),
- myStartupInfo));
+ &myStartupInfo));
TraceLoggingWrite(g_hConhostV2EventTraceProvider,
"SrvInit_DelegateToTerminalSucceeded",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
- inPipeTheirSide.reset();
- outPipeTheirSide.reset();
signalPipeTheirSide.reset();
// GH#13211 - Make sure the terminal obeys the resizing quirk. Otherwise,
diff --git a/src/inc/til/u8u16convert.h b/src/inc/til/u8u16convert.h
index 69ee6b0558..b026220414 100644
--- a/src/inc/til/u8u16convert.h
+++ b/src/inc/til/u8u16convert.h
@@ -26,7 +26,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
// state structure for maintenance of UTF-8 partials
struct u8state
{
- char partials[4];
+ char partials[4]{};
uint8_t have{};
uint8_t want{};