diff --git a/src/host/input.cpp b/src/host/input.cpp index fd92154020c..952d3a02915 100644 --- a/src/host/input.cpp +++ b/src/host/input.cpp @@ -321,28 +321,35 @@ void ProcessCtrlEvents() return; } - auto Status = STATUS_SUCCESS; + const auto ctrl = ServiceLocator::LocateConsoleControl(); + for (const auto& r : termRecords) { - /* - * Status will be non-successful if a process attached to this console - * vetoes shutdown. In that case, we don't want to try to kill any more - * processes, but we do need to make sure we continue looping so we - * can close any remaining process handles. The exception is if the - * process is inaccessible, such that we can't even open a handle for - * query. In this case, use best effort to send the close event but - * ignore any errors. - */ - if (SUCCEEDED_NTSTATUS(Status)) - { - Status = ServiceLocator::LocateConsoleControl() - ->EndTask(r.dwProcessID, - EventType, - CtrlFlags); - if (!r.hProcess) - { - Status = STATUS_SUCCESS; - } - } + // Older versions of Windows would do various things if the EndTask() call failed: + // * XP: Pops up a "Windows can't end this program" dialog for every already-dead process. + // * Vista - Win 7: Simply skips over already-dead processes. + // * Win 8 - Win 11 26100: Aborts on an already-dead process (you have to WM_CLOSE conhost multiple times). + // + // That last period had the following comment: + // Status will be non-successful if a process attached to this console + // vetoes shutdown. In that case, we don't want to try to kill any more + // processes, but we do need to make sure we continue looping so we + // can close any remaining process handles. The exception is if the + // process is inaccessible, such that we can't even open a handle for + // query. In this case, use best effort to send the close event but + // ignore any errors. + // + // The corresponding logic worked like this: + // if (FAILED(EndTask(...)) && r.hProcess) { + // break; + // } + // + // That logic was removed around the Windows 11 26100 time frame, because CSRSS + // (which handles EndTask) now waits 5s and then force-kills the process for us. + // Going back to the Win 7 behavior then should make shutdown a lot more robust. + // The bad news is that EndTask() returns STATUS_UNSUCCESSFUL no matter whether + // the process was already dead, or if the request actually failed for some reason. + // Hopefully there aren't any regressions, but we can't know without trying. + LOG_IF_NTSTATUS_FAILED(ctrl->EndTask(r.dwProcessID, EventType, CtrlFlags)); } } diff --git a/src/server/ProcessList.cpp b/src/server/ProcessList.cpp index e304556692a..e01e20bb8de 100644 --- a/src/server/ProcessList.cpp +++ b/src/server/ProcessList.cpp @@ -211,9 +211,15 @@ ConsoleProcessHandle* ConsoleProcessList::GetOldestProcess() const { termRecords.clear(); + // The caller (ProcessCtrlEvents) expects them in newest-to-oldest order, + // because that's how Windows has historically always dispatched these events. + auto it = _processes.crbegin(); + const auto end = _processes.crend(); + // Dig through known processes looking for a match - for (const auto& p : _processes) + for (; it != end; ++it) { + const auto p = *it; // If no limit was specified OR if we have a match, generate a new termination record. if (!dwLimitingProcessId || p->_ulProcessGroupId == dwLimitingProcessId)