Skip to content

Commit 391abaf

Browse files
authored
Refactor ConsoleProcessList (#14421)
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. ## Validation Steps Performed * All unit/feature tests pass ✅ * Launching a TUI application inside pwsh inside cmd and exiting kills all 3 applications ✅
1 parent 3c78e01 commit 391abaf

File tree

8 files changed

+146
-206
lines changed

8 files changed

+146
-206
lines changed

src/host/ft_fuzzer/fuzzmain.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ struct NullDeviceComm : public IDeviceComm
7474
RETURN_IF_FAILED(gci.ProcessHandleList.AllocProcessData(GetCurrentProcessId(),
7575
GetCurrentThreadId(),
7676
0,
77-
nullptr,
7877
&pProcessHandle));
7978
pProcessHandle->fRootProcess = true;
8079

src/host/input.cpp

+15-25
Original file line numberDiff line numberDiff line change
@@ -275,27 +275,21 @@ void ProcessCtrlEvents()
275275
const auto LimitingProcessId = gci.LimitingProcessId;
276276
gci.LimitingProcessId = 0;
277277

278-
ConsoleProcessTerminationRecord* rgProcessHandleList;
279-
size_t cProcessHandleList;
280-
281-
auto hr = gci.ProcessHandleList
282-
.GetTerminationRecordsByGroupId(LimitingProcessId,
283-
WI_IsFlagSet(gci.CtrlFlags,
284-
CONSOLE_CTRL_CLOSE_FLAG),
285-
&rgProcessHandleList,
286-
&cProcessHandleList);
287-
288-
if (FAILED(hr) || cProcessHandleList == 0)
278+
std::vector<ConsoleProcessTerminationRecord> termRecords;
279+
const auto hr = gci.ProcessHandleList
280+
.GetTerminationRecordsByGroupId(LimitingProcessId,
281+
WI_IsFlagSet(gci.CtrlFlags,
282+
CONSOLE_CTRL_CLOSE_FLAG),
283+
termRecords);
284+
285+
if (FAILED(hr) || termRecords.empty())
289286
{
290287
gci.UnlockConsole();
291288
return;
292289
}
293290

294291
// Copy ctrl flags.
295-
auto CtrlFlags = gci.CtrlFlags;
296-
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)))));
297-
298-
gci.CtrlFlags = 0;
292+
const auto CtrlFlags = std::exchange(gci.CtrlFlags, 0);
299293

300294
gci.UnlockConsole();
301295

@@ -330,10 +324,13 @@ void ProcessCtrlEvents()
330324
case CONSOLE_CTRL_SHUTDOWN_FLAG:
331325
EventType = CTRL_SHUTDOWN_EVENT;
332326
break;
327+
328+
default:
329+
return;
333330
}
334331

335332
auto Status = STATUS_SUCCESS;
336-
for (size_t i = 0; i < cProcessHandleList; i++)
333+
for (const auto& r : termRecords)
337334
{
338335
/*
339336
* Status will be non-successful if a process attached to this console
@@ -347,20 +344,13 @@ void ProcessCtrlEvents()
347344
if (NT_SUCCESS(Status))
348345
{
349346
Status = ServiceLocator::LocateConsoleControl()
350-
->EndTask(rgProcessHandleList[i].dwProcessID,
347+
->EndTask(r.dwProcessID,
351348
EventType,
352349
CtrlFlags);
353-
if (rgProcessHandleList[i].hProcess == nullptr)
350+
if (!r.hProcess)
354351
{
355352
Status = STATUS_SUCCESS;
356353
}
357354
}
358-
359-
if (rgProcessHandleList[i].hProcess != nullptr)
360-
{
361-
CloseHandle(rgProcessHandleList[i].hProcess);
362-
}
363355
}
364-
365-
delete[] rgProcessHandleList;
366356
}

src/interactivity/win32/windowio.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pP
7878
else
7979
{
8080
// Find a process to own the console window. If there are none then let's use conhost's.
81-
pProcessData = gci.ProcessHandleList.FindProcessInList(ConsoleProcessList::ROOT_PROCESS_ID);
81+
pProcessData = gci.ProcessHandleList.GetRootProcess();
8282
if (!pProcessData)
8383
{
8484
// No root process ID? Pick the oldest existing process.
85-
pProcessData = gci.ProcessHandleList.GetFirstProcess();
85+
pProcessData = gci.ProcessHandleList.GetOldestProcess();
8686
}
8787

8888
if (pProcessData != nullptr)
@@ -986,7 +986,7 @@ LRESULT CALLBACK DialogHookProc(int nCode, WPARAM /*wParam*/, LPARAM lParam)
986986
NTSTATUS InitWindowsSubsystem(_Out_ HHOOK* phhook)
987987
{
988988
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
989-
auto ProcessData = gci.ProcessHandleList.FindProcessInList(ConsoleProcessList::ROOT_PROCESS_ID);
989+
auto ProcessData = gci.ProcessHandleList.GetRootProcess();
990990
FAIL_FAST_IF(!(ProcessData != nullptr && ProcessData->fRootProcess));
991991

992992
// Create and activate the main window

src/server/ApiDispatchersInternal.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ using Microsoft::Console::Interactivity::ServiceLocator;
9393
RETURN_IF_FAILED(gci.ProcessHandleList.AllocProcessData(a->ProcessGroupId,
9494
0,
9595
a->ProcessGroupId,
96-
ProcessHandle,
9796
nullptr));
9897
}
9998
}

src/server/IoDispatchers.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,6 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
452452
Status = NTSTATUS_FROM_HRESULT(gci.ProcessHandleList.AllocProcessData(dwProcessId,
453453
dwThreadId,
454454
Cac.ProcessGroupId,
455-
nullptr,
456455
&ProcessData));
457456

458457
if (!NT_SUCCESS(Status))

src/server/ProcessHandle.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ Revision History:
2828
class ConsoleProcessHandle
2929
{
3030
public:
31+
ConsoleProcessHandle(const DWORD dwProcessId,
32+
const DWORD dwThreadId,
33+
const ULONG ulProcessGroupId);
34+
~ConsoleProcessHandle() = default;
35+
ConsoleProcessHandle(const ConsoleProcessHandle&) = delete;
36+
ConsoleProcessHandle(ConsoleProcessHandle&&) = delete;
37+
ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete;
38+
ConsoleProcessHandle& operator=(ConsoleProcessHandle&&) & = delete;
39+
3140
const std::unique_ptr<ConsoleWaitQueue> pWaitBlockQueue;
3241
std::unique_ptr<ConsoleHandleData> pInputHandle;
3342
std::unique_ptr<ConsoleHandleData> pOutputHandle;
@@ -47,15 +56,6 @@ class ConsoleProcessHandle
4756
const ULONG64 GetProcessCreationTime() const;
4857

4958
private:
50-
ConsoleProcessHandle(const DWORD dwProcessId,
51-
const DWORD dwThreadId,
52-
const ULONG ulProcessGroupId);
53-
~ConsoleProcessHandle() = default;
54-
ConsoleProcessHandle(const ConsoleProcessHandle&) = delete;
55-
ConsoleProcessHandle(ConsoleProcessHandle&&) = delete;
56-
ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete;
57-
ConsoleProcessHandle& operator=(ConsoleProcessHandle&&) & = delete;
58-
5959
ULONG _ulTerminateCount;
6060
ULONG const _ulProcessGroupId;
6161
wil::unique_handle const _hProcess;

0 commit comments

Comments
 (0)