Skip to content

Commit

Permalink
This is far simpler
Browse files Browse the repository at this point in the history
  • Loading branch information
zadjii-msft committed Jun 9, 2022
1 parent b1721f6 commit 55a7331
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 60 deletions.
9 changes: 4 additions & 5 deletions src/interactivity/base/HostSignalInputThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,11 @@ T HostSignalInputThread::_ReceiveTypedPacket()
}
case HostSignals::SetForeground:
{
auto msg = _ReceiveTypedPacket<HostSignalSetForegroundData>();
_ReceiveTypedPacket<HostSignalSetForegroundData>();

// 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;
}
Expand Down
10 changes: 4 additions & 6 deletions src/interactivity/base/RemoteConsoleControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,10 @@ template<typename T>

[[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)
Expand Down
3 changes: 3 additions & 0 deletions src/interactivity/base/RemoteConsoleControl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Author(s):
#pragma once

#include "../inc/IConsoleControl.hpp"
#include "../win32/ConsoleControl.hpp"

namespace Microsoft::Console::Interactivity
{
Expand All @@ -29,5 +30,7 @@ namespace Microsoft::Console::Interactivity

private:
wil::unique_handle _pipe;

Win32::ConsoleControl _control;
};
}
41 changes: 0 additions & 41 deletions src/server/ProcessHandle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions src/server/ProcessHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -59,7 +59,6 @@ class ConsoleProcessHandle
ULONG _ulTerminateCount;
ULONG const _ulProcessGroupId;
wil::unique_handle const _hProcess;
wil::unique_handle _hProcessInConhost;

mutable ULONG64 _processCreationTime;

Expand Down
8 changes: 2 additions & 6 deletions src/server/ProcessList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 55a7331

Please sign in to comment.