Skip to content

Commit 2f1d2ea

Browse files
lheckerDHowett
authored andcommitted
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 ✅ (cherry picked from commit 391abaf) Service-Card-Id: 87207825 Service-Version: 1.16
1 parent a7b0fdf commit 2f1d2ea

File tree

8 files changed

+146
-206
lines changed

8 files changed

+146
-206
lines changed

src/host/ft_fuzzer/fuzzmain.cpp

Lines changed: 0 additions & 1 deletion
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

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -274,27 +274,21 @@ void ProcessCtrlEvents()
274274
const auto LimitingProcessId = gci.LimitingProcessId;
275275
gci.LimitingProcessId = 0;
276276

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

293290
// Copy ctrl flags.
294-
auto CtrlFlags = gci.CtrlFlags;
295-
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)))));
296-
297-
gci.CtrlFlags = 0;
291+
const auto CtrlFlags = std::exchange(gci.CtrlFlags, 0);
298292

299293
gci.UnlockConsole();
300294

@@ -329,10 +323,13 @@ void ProcessCtrlEvents()
329323
case CONSOLE_CTRL_SHUTDOWN_FLAG:
330324
EventType = CTRL_SHUTDOWN_EVENT;
331325
break;
326+
327+
default:
328+
return;
332329
}
333330

334331
auto Status = STATUS_SUCCESS;
335-
for (size_t i = 0; i < cProcessHandleList; i++)
332+
for (const auto& r : termRecords)
336333
{
337334
/*
338335
* Status will be non-successful if a process attached to this console
@@ -346,20 +343,13 @@ void ProcessCtrlEvents()
346343
if (NT_SUCCESS(Status))
347344
{
348345
Status = ServiceLocator::LocateConsoleControl()
349-
->EndTask(rgProcessHandleList[i].dwProcessID,
346+
->EndTask(r.dwProcessID,
350347
EventType,
351348
CtrlFlags);
352-
if (rgProcessHandleList[i].hProcess == nullptr)
349+
if (!r.hProcess)
353350
{
354351
Status = STATUS_SUCCESS;
355352
}
356353
}
357-
358-
if (rgProcessHandleList[i].hProcess != nullptr)
359-
{
360-
CloseHandle(rgProcessHandleList[i].hProcess);
361-
}
362354
}
363-
364-
delete[] rgProcessHandleList;
365355
}

src/interactivity/win32/windowio.cpp

Lines changed: 3 additions & 3 deletions
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

Lines changed: 0 additions & 1 deletion
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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,6 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
435435
Status = NTSTATUS_FROM_HRESULT(gci.ProcessHandleList.AllocProcessData(dwProcessId,
436436
dwThreadId,
437437
Cac.ProcessGroupId,
438-
nullptr,
439438
&ProcessData));
440439

441440
if (!NT_SUCCESS(Status))

src/server/ProcessHandle.h

Lines changed: 9 additions & 9 deletions
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)