Refactor ConsoleProcessList

src/server/ApiDispatchersInternal.cpp

@@ -90,17 +90,10 @@ using Microsoft::Console::Interactivity::ServiceLocator;
             RETURN_IF_NTSTATUS_FAILED((NtPrivApi::s_GetProcessParentId(&ProcessId)));
             ProcessHandle = gci.ProcessHandleList.FindProcessInList(ProcessId);
             RETURN_HR_IF_NULL(E_INVALIDARG, ProcessHandle);
-
-            const auto hr = gci.ProcessHandleList.AllocProcessData(a->ProcessGroupId,
-                                                                   0,
-                                                                   a->ProcessGroupId,
-                                                                   nullptr);
-            // AllocProcessData returns E_FAIL if the given process ID already exists.
-            // The process ID already existing isn't really an error though so we ignore it.
-            if (FAILED(hr) && hr != E_FAIL)
-            {
-                RETURN_HR(hr);
-            }
+            RETURN_IF_FAILED(gci.ProcessHandleList.AllocProcessData(a->ProcessGroupId,
+                                                                    0,
+                                                                    a->ProcessGroupId,
+                                                                    nullptr));
         }
     }
 

src/server/ProcessHandle.h

@@ -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;
@@ -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;

src/server/ProcessList.cpp

@@ -22,7 +22,7 @@ using namespace Microsoft::Console::Interactivity;
 //                 - If not used, return code will specify whether this process is known to the list or not.
 // Return Value:
 // - S_OK if the process was recorded in the list successfully or already existed.
-// - E_FAIL if we're running into an LPC port conflict by nature of the process chain.
+// - S_FALSE if we're running into an LPC port conflict by nature of the process chain.
 // - E_OUTOFMEMORY if there wasn't space to allocate a handle or push it into the list.
 [[nodiscard]] HRESULT ConsoleProcessList::AllocProcessData(const DWORD dwProcessId,
                                                            const DWORD dwThreadId,
@@ -31,23 +31,20 @@ using namespace Microsoft::Console::Interactivity;
 {
     assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());
 
-    auto pProcessData = FindProcessInList(dwProcessId);
-    if (pProcessData)
+    if (FindProcessInList(dwProcessId))
     {
-        return E_FAIL;
+        return S_FALSE;
     }
 
+    std::unique_ptr<ConsoleProcessHandle> pProcessData;
     try
     {
-        pProcessData = new ConsoleProcessHandle{ dwProcessId, dwThreadId, ulProcessGroupId };
-        _processes.emplace_back(pProcessData);
+        pProcessData = std::make_unique<ConsoleProcessHandle>(dwProcessId, dwThreadId, ulProcessGroupId);
+        _processes.emplace_back(pProcessData.get());
     }
     CATCH_RETURN();
 
-    if (ppProcessData)
-    {
-        *ppProcessData = pProcessData;
-    }
+    wil::assign_to_opt_param(ppProcessData, pProcessData.release());
 
     return S_OK;
 }
@@ -138,6 +135,26 @@ ConsoleProcessHandle* ConsoleProcessList::GetRootProcess() const
     return nullptr;
 }
 
+// Routine Description:
+// - Gets the first process in the list.
+// - Used for reassigning a new root process.
+// TODO: MSFT 9450737 - encapsulate root process logic. https://osgvsowi/9450737
+// Arguments:
+// - <none>
+// Return Value:
+// - Pointer to the first item in the list or nullptr if there are no items.
+ConsoleProcessHandle* ConsoleProcessList::GetOldestProcess() const
+{
+    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());
+
+    if (!_processes.empty())
+    {
+        return _processes.front();
+    }
+
+    return nullptr;
+}
+
 // Routine Description:
 // - Retrieves the entire list of process IDs that is known to this list.
 // Arguments:
@@ -230,26 +247,6 @@ ConsoleProcessHandle* ConsoleProcessList::GetRootProcess() const
     CATCH_RETURN();
 }
 
-// Routine Description:
-// - Gets the first process in the list.
-// - Used for reassigning a new root process.
-// TODO: MSFT 9450737 - encapsulate root process logic. https://osgvsowi/9450737
-// Arguments:
-// - <none>
-// Return Value:
-// - Pointer to the first item in the list or nullptr if there are no items.
-ConsoleProcessHandle* ConsoleProcessList::GetOldestProcess() const
-{
-    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());
-
-    if (!_processes.empty())
-    {
-        return _processes.front();
-    }
-
-    return nullptr;
-}
-
 // Routine Description:
 // - Requests that the OS change the process priority for the console and all attached client processes
 // Arguments:

src/server/ProcessList.h

@@ -44,13 +44,12 @@ class ConsoleProcessList
     ConsoleProcessHandle* FindProcessInList(const DWORD dwProcessId) const;
     ConsoleProcessHandle* FindProcessByGroupId(_In_ ULONG ulProcessGroupId) const;
     ConsoleProcessHandle* GetRootProcess() const;
+    ConsoleProcessHandle* GetOldestProcess() const;
 
     [[nodiscard]] HRESULT GetTerminationRecordsByGroupId(const DWORD dwLimitingProcessId,
                                                          const bool fCtrlClose,
                                                          std::vector<ConsoleProcessTerminationRecord>& termRecords) const;
 
-    ConsoleProcessHandle* GetOldestProcess() const;
-
     [[nodiscard]] HRESULT GetProcessList(_Inout_updates_(*pcProcessList) DWORD* const pProcessList,
                                          _Inout_ size_t* const pcProcessList) const;