Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor ConsoleProcessList #14421

Merged
6 commits merged into from
Dec 2, 2022
Merged

Refactor ConsoleProcessList #14421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
76a1367
Refactor ConsoleProcessList
lhecker Nov 22, 2022
e1bb5ef
Merge remote-tracking branch 'origin/main' into dev/lhecker/console-p…
lhecker Nov 30, 2022
2aef629
Address feedback
lhecker Nov 30, 2022
6724ed1
Merge remote-tracking branch 'origin/main' into dev/lhecker/console-p…
lhecker Dec 1, 2022
1b967c3
Fix spelling
lhecker Dec 1, 2022
c686b11
Address feedback
lhecker Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/host/ft_fuzzer/fuzzmain.cpp
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,27 +275,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)))));
Copy link
Member Author

@lhecker lhecker Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no good reason for this to fail-fast. We can just add a default: case to the switch case below, which I now did.


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

gci.UnlockConsole();

Expand Down Expand Up @@ -330,10 +324,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 @@ -347,20 +344,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);
}
Comment on lines -359 to -362
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still need this in the event that if (NT_SUCCESS(Status)) fails?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it so that the hProcess is a wil::unique_handle so it'll get cleaned up automatically.

}

delete[] rgProcessHandleList;
}
6 changes: 3 additions & 3 deletions src/interactivity/win32/windowio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ 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.FindProcessInList(ConsoleProcessList::ROOT_PROCESS_ID);
pProcessData = gci.ProcessHandleList.GetRootProcess();
if (!pProcessData)
{
// No root process ID? Pick the oldest existing process.
pProcessData = gci.ProcessHandleList.GetFirstProcess();
pProcessData = gci.ProcessHandleList.GetOldestProcess();
lhecker marked this conversation as resolved.
Show resolved Hide resolved
}

if (pProcessData != nullptr)
Expand Down Expand Up @@ -986,7 +986,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,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
View file
Open in desktop
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