diff --git a/src/interactivity/base/HostSignalInputThread.cpp b/src/interactivity/base/HostSignalInputThread.cpp index 82f982595bb..bed01bbd013 100644 --- a/src/interactivity/base/HostSignalInputThread.cpp +++ b/src/interactivity/base/HostSignalInputThread.cpp @@ -94,12 +94,11 @@ T HostSignalInputThread::_ReceiveTypedPacket() } case HostSignals::SetForeground: { - auto msg = _ReceiveTypedPacket(); + _ReceiveTypedPacket(); - // GH#13211 - The OpenConsole side should have already duplicated - // the handle to us, and processId should be the value in our - // process space. - LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->SetForeground(ULongToHandle(msg.processId), msg.isForeground)); + // GH#13211 - This honestly shouldn't be called by OpenConsole + // anymore, but it's possible that a much older version of + // OpenConsole is still calling this. Just do nothing. break; } diff --git a/src/interactivity/base/RemoteConsoleControl.cpp b/src/interactivity/base/RemoteConsoleControl.cpp index 0b9283ba8bb..7cd03e36317 100644 --- a/src/interactivity/base/RemoteConsoleControl.cpp +++ b/src/interactivity/base/RemoteConsoleControl.cpp @@ -57,12 +57,10 @@ template [[nodiscard]] NTSTATUS RemoteConsoleControl::SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground) { - HostSignalSetForegroundData data{}; - data.sizeInBytes = sizeof(data); - data.processId = HandleToULong(hProcess); - data.isForeground = fForeground; - - return _SendTypedPacket(_pipe.get(), HostSignals::SetForeground, data); + // GH#13211 - Apparently this API doesn't need to be forwarded to conhost at + // all. Instead, just perform the ConsoleControl operation here, in proc. + // This lets us avoid all sorts of strange handle duplicating weirdness. + return _control.SetForeground(hProcess, fForeground); } [[nodiscard]] NTSTATUS RemoteConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) diff --git a/src/interactivity/base/RemoteConsoleControl.hpp b/src/interactivity/base/RemoteConsoleControl.hpp index ee22aeef4dc..b922b9beaa6 100644 --- a/src/interactivity/base/RemoteConsoleControl.hpp +++ b/src/interactivity/base/RemoteConsoleControl.hpp @@ -14,6 +14,7 @@ Author(s): #pragma once #include "../inc/IConsoleControl.hpp" +#include "../win32/ConsoleControl.hpp" namespace Microsoft::Console::Interactivity { @@ -29,5 +30,7 @@ namespace Microsoft::Console::Interactivity private: wil::unique_handle _pipe; + + Win32::ConsoleControl _control; }; } diff --git a/src/server/ProcessHandle.cpp b/src/server/ProcessHandle.cpp index 09229f9409e..7c2adfc6417 100644 --- a/src/server/ProcessHandle.cpp +++ b/src/server/ProcessHandle.cpp @@ -8,9 +8,6 @@ #include "../host/globals.h" #include "../host/telemetry.hpp" -#include "../interactivity/inc/ServiceLocator.hpp" - -using namespace Microsoft::Console::Interactivity; // Routine Description: // - Constructs an instance of the ConsoleProcessHandle Class // - NOTE: Can throw if allocation fails or if there is a console policy we do not understand. @@ -37,44 +34,6 @@ ConsoleProcessHandle::ConsoleProcessHandle(const DWORD dwProcessId, { Telemetry::Instance().LogProcessConnected(_hProcess.get()); } - - // GH#13211 - If we're running as the delegation console (someone handed off - // to us), then we need to make sure the original conhost has access to this - // process handle as well. Otherwise, the future calls to - // ConsoleControl(SetForeground,...) won't work, because the literal handle - // value doesn't exist in the original conhost process space. - // * g.handoffInboxConsoleHandle is only set when we've been delegated to - // * We can't just pass something like the PID, because the OS conhost - // already expects a literal handle value via the HostSignalInputThread. - // If we changed that in the OpenConsole version, then there'll surely be - // the chance for a mispatch between the OS conhost and the delegated - // console. - if (const auto& conhost{ ServiceLocator::LocateGlobals().handoffInboxConsoleHandle }) - { - LOG_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(), - _hProcess.get(), - conhost.get(), - _hProcessInConhost.put(), - 0 /*dwDesiredAccess, ignored*/, - false, - DUPLICATE_SAME_ACCESS)); - } -} - -ConsoleProcessHandle::~ConsoleProcessHandle() -{ - // Close out the handle in the origin conhost. - const auto& conhost{ ServiceLocator::LocateGlobals().handoffInboxConsoleHandle }; - if (_hProcessInConhost && conhost) - { - LOG_IF_WIN32_BOOL_FALSE(DuplicateHandle(conhost.get(), - _hProcessInConhost.get(), - nullptr /* hTargetProcessHandle */, - nullptr /* lpTargetHandle, ignored */, - 0 /* dwDesiredAccess, ignored */, - false, - DUPLICATE_CLOSE_SOURCE)); - } } // Routine Description: diff --git a/src/server/ProcessHandle.h b/src/server/ProcessHandle.h index 5772fd12650..50084f45430 100644 --- a/src/server/ProcessHandle.h +++ b/src/server/ProcessHandle.h @@ -50,7 +50,7 @@ class ConsoleProcessHandle ConsoleProcessHandle(const DWORD dwProcessId, const DWORD dwThreadId, const ULONG ulProcessGroupId); - ~ConsoleProcessHandle(); + ~ConsoleProcessHandle() = default; ConsoleProcessHandle(const ConsoleProcessHandle&) = delete; ConsoleProcessHandle(ConsoleProcessHandle&&) = delete; ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete; @@ -59,7 +59,6 @@ class ConsoleProcessHandle ULONG _ulTerminateCount; ULONG const _ulProcessGroupId; wil::unique_handle const _hProcess; - wil::unique_handle _hProcessInConhost; mutable ULONG64 _processCreationTime; diff --git a/src/server/ProcessList.cpp b/src/server/ProcessList.cpp index 5051e305694..ab9c529a76d 100644 --- a/src/server/ProcessList.cpp +++ b/src/server/ProcessList.cpp @@ -305,13 +305,9 @@ void ConsoleProcessList::ModifyConsoleProcessFocus(const bool fForeground) { const auto pProcessHandle = *it; - // If we're a delegated-to console (OpenConsole), then make sure to pass - // in the value of the handle in the origin conhost, not our own. - if (const auto& handleToUse = pProcessHandle->_hProcessInConhost ? - pProcessHandle->_hProcessInConhost : - pProcessHandle->_hProcess) + if (pProcessHandle->_hProcess) { - _ModifyProcessForegroundRights(handleToUse.get(), fForeground); + _ModifyProcessForegroundRights(pProcessHandle->_hProcess.get(), fForeground); } it = std::next(it);