Skip to content

Commit

Permalink
Refactor ConsoleProcessList (#14421)
Browse files Browse the repository at this point in the history
This commit is just a slight refactor of `ConsoleProcessList` which I've noticed
was in a poor shape. It replaces iterators with for-range loops, etc.

Additionally this fixes a bug in `SetConsoleWindowOwner`, where it used to
default to the newest client process instead of the oldest.

Finally, it changes the process container type from a doubly linked list
over to a simple array/vector, because using linked lists for heap allocated
elements felt quite a bit silly. To preserve the previous behavior of
`GetProcessList`, it simply iterates through the vector backwards.

* All unit/feature tests pass ✅
* Launching a TUI application inside pwsh inside cmd
  and exiting kills all 3 applications ✅

(cherry picked from commit 391abaf)
Service-Card-Id: 87207823
Service-Version: 1.15
  • Loading branch information
lhecker authored and DHowett committed Dec 12, 2022
1 parent d4853a8 commit 111a6c0
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 205 deletions.
1 change: 0 additions & 1 deletion src/host/ft_fuzzer/fuzzmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ struct NullDeviceComm : public IDeviceComm
RETURN_IF_FAILED(gci.ProcessHandleList.AllocProcessData(GetCurrentProcessId(),
GetCurrentThreadId(),
0,
nullptr,
&pProcessHandle));
pProcessHandle->fRootProcess = true;

Expand Down
40 changes: 15 additions & 25 deletions src/host/input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,27 +274,21 @@ void ProcessCtrlEvents()
const auto LimitingProcessId = gci.LimitingProcessId;
gci.LimitingProcessId = 0;

ConsoleProcessTerminationRecord* rgProcessHandleList;
size_t cProcessHandleList;

auto hr = gci.ProcessHandleList
.GetTerminationRecordsByGroupId(LimitingProcessId,
WI_IsFlagSet(gci.CtrlFlags,
CONSOLE_CTRL_CLOSE_FLAG),
&rgProcessHandleList,
&cProcessHandleList);

if (FAILED(hr) || cProcessHandleList == 0)
std::vector<ConsoleProcessTerminationRecord> termRecords;
const auto hr = gci.ProcessHandleList
.GetTerminationRecordsByGroupId(LimitingProcessId,
WI_IsFlagSet(gci.CtrlFlags,
CONSOLE_CTRL_CLOSE_FLAG),
termRecords);

if (FAILED(hr) || termRecords.empty())
{
gci.UnlockConsole();
return;
}

// Copy ctrl flags.
auto CtrlFlags = gci.CtrlFlags;
FAIL_FAST_IF(!(!((CtrlFlags & (CONSOLE_CTRL_CLOSE_FLAG | CONSOLE_CTRL_BREAK_FLAG | CONSOLE_CTRL_C_FLAG)) && (CtrlFlags & (CONSOLE_CTRL_LOGOFF_FLAG | CONSOLE_CTRL_SHUTDOWN_FLAG)))));

gci.CtrlFlags = 0;
const auto CtrlFlags = std::exchange(gci.CtrlFlags, 0);

gci.UnlockConsole();

Expand Down Expand Up @@ -329,10 +323,13 @@ void ProcessCtrlEvents()
case CONSOLE_CTRL_SHUTDOWN_FLAG:
EventType = CTRL_SHUTDOWN_EVENT;
break;

default:
return;
}

auto Status = STATUS_SUCCESS;
for (size_t i = 0; i < cProcessHandleList; i++)
for (const auto& r : termRecords)
{
/*
* Status will be non-successful if a process attached to this console
Expand All @@ -346,20 +343,13 @@ void ProcessCtrlEvents()
if (NT_SUCCESS(Status))
{
Status = ServiceLocator::LocateConsoleControl()
->EndTask(rgProcessHandleList[i].dwProcessID,
->EndTask(r.dwProcessID,
EventType,
CtrlFlags);
if (rgProcessHandleList[i].hProcess == nullptr)
if (!r.hProcess)
{
Status = STATUS_SUCCESS;
}
}

if (rgProcessHandleList[i].hProcess != nullptr)
{
CloseHandle(rgProcessHandleList[i].hProcess);
}
}

delete[] rgProcessHandleList;
}
10 changes: 8 additions & 2 deletions src/interactivity/win32/windowio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,13 @@ VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pP
else
{
// Find a process to own the console window. If there are none then let's use conhost's.
pProcessData = gci.ProcessHandleList.GetFirstProcess();
pProcessData = gci.ProcessHandleList.GetRootProcess();
if (!pProcessData)
{
// No root process ID? Pick the oldest existing process.
pProcessData = gci.ProcessHandleList.GetOldestProcess();
}

if (pProcessData != nullptr)
{
dwProcessId = pProcessData->dwProcessId;
Expand Down Expand Up @@ -988,7 +994,7 @@ LRESULT CALLBACK DialogHookProc(int nCode, WPARAM /*wParam*/, LPARAM lParam)
NTSTATUS InitWindowsSubsystem(_Out_ HHOOK* phhook)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto ProcessData = gci.ProcessHandleList.FindProcessInList(ConsoleProcessList::ROOT_PROCESS_ID);
auto ProcessData = gci.ProcessHandleList.GetRootProcess();
FAIL_FAST_IF(!(ProcessData != nullptr && ProcessData->fRootProcess));

// Create and activate the main window
Expand Down
1 change: 0 additions & 1 deletion src/server/ApiDispatchersInternal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ using Microsoft::Console::Interactivity::ServiceLocator;
RETURN_IF_FAILED(gci.ProcessHandleList.AllocProcessData(a->ProcessGroupId,
0,
a->ProcessGroupId,
ProcessHandle,
nullptr));
}
}
Expand Down
1 change: 0 additions & 1 deletion src/server/IoDispatchers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,6 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
Status = NTSTATUS_FROM_HRESULT(gci.ProcessHandleList.AllocProcessData(dwProcessId,
dwThreadId,
Cac.ProcessGroupId,
nullptr,
&ProcessData));

if (!NT_SUCCESS(Status))
Expand Down
18 changes: 9 additions & 9 deletions src/server/ProcessHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ Revision History:
class ConsoleProcessHandle
{
public:
ConsoleProcessHandle(const DWORD dwProcessId,
const DWORD dwThreadId,
const ULONG ulProcessGroupId);
~ConsoleProcessHandle() = default;
ConsoleProcessHandle(const ConsoleProcessHandle&) = delete;
ConsoleProcessHandle(ConsoleProcessHandle&&) = delete;
ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete;
ConsoleProcessHandle& operator=(ConsoleProcessHandle&&) & = delete;

const std::unique_ptr<ConsoleWaitQueue> pWaitBlockQueue;
std::unique_ptr<ConsoleHandleData> pInputHandle;
std::unique_ptr<ConsoleHandleData> pOutputHandle;
Expand All @@ -47,15 +56,6 @@ class ConsoleProcessHandle
const ULONG64 GetProcessCreationTime() const;

private:
ConsoleProcessHandle(const DWORD dwProcessId,
const DWORD dwThreadId,
const ULONG ulProcessGroupId);
~ConsoleProcessHandle() = default;
ConsoleProcessHandle(const ConsoleProcessHandle&) = delete;
ConsoleProcessHandle(ConsoleProcessHandle&&) = delete;
ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete;
ConsoleProcessHandle& operator=(ConsoleProcessHandle&&) & = delete;

ULONG _ulTerminateCount;
ULONG const _ulProcessGroupId;
wil::unique_handle const _hProcess;
Expand Down
Loading

0 comments on commit 111a6c0

Please sign in to comment.