Repository: microsoft/terminal
File: src/server/ProcessList.cpp (C++)

Code before:
//                 - 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.
// - 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,
                                                           const ULONG ulProcessGroupId,
                                                           _Outptr_opt_ ConsoleProcessHandle** const ppProcessData)
{
    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());

    auto pProcessData = FindProcessInList(dwProcessId);
    if (pProcessData)
    {
        return E_FAIL;
    }

    try
    {
        pProcessData = new ConsoleProcessHandle{ dwProcessId, dwThreadId, ulProcessGroupId };
        _processes.emplace_back(pProcessData);
    }
    CATCH_RETURN();

    if (ppProcessData)
    {
        *ppProcessData = pProcessData;
    }

    return S_OK;
}

// Routine Description:
// - This routine frees any per-process data allocated by the console.
// Arguments:
// - pProcessData - Pointer to the per-process data structure.
// Return Value:
// - <none>
void ConsoleProcessList::FreeProcessData(_In_ ConsoleProcessHandle* const pProcessData)
{
    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());

    const auto it = std::ranges::find(_processes, pProcessData);
    if (it != _processes.end())
    {
        _processes.erase(it);
    }
    else
    {
        assert(false);

Review comment (bug):
There's only two callers. `ApiDispatchers::ServerGenerateConsoleCtrlEvent` always calls this with a valid `pParentProcessData` and won't ever run into this condition. `IoDispatchers::ConsoleHandleConnectionRequest` calls this method with a `nullptr` instead. This indicates that this error triggers, because we're getting redundant connection requests for the same process ID.

Code after:
//                 - 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.
// - 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,
                                                           const ULONG ulProcessGroupId,
                                                           _Outptr_opt_ ConsoleProcessHandle** const ppProcessData)
{
    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());

    if (FindProcessInList(dwProcessId))
    {
        return S_FALSE;
    }

    std::unique_ptr<ConsoleProcessHandle> pProcessData;
    try
    {
        pProcessData = std::make_unique<ConsoleProcessHandle>(dwProcessId, dwThreadId, ulProcessGroupId);
        _processes.emplace_back(pProcessData.get());
    }
    CATCH_RETURN();

    wil::assign_to_opt_param(ppProcessData, pProcessData.release());

    return S_OK;
}

// Routine Description:
// - This routine frees any per-process data allocated by the console.
// Arguments:
// - pProcessData - Pointer to the per-process data structure.
// Return Value:
// - <none>
void ConsoleProcessList::FreeProcessData(_In_ ConsoleProcessHandle* const pProcessData)
{
    assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked());

    const auto it = std::ranges::find(_processes, pProcessData);
    if (it != _processes.end())
    {
        _processes.erase(it);
        delete pProcessData;
    }
    else
    {
        // The pointer not existing in the process list would be similar to a heap corruption,
        // as the only code allowed to allocate a `ConsoleProcessHandle` is us, in AllocProcessData().
        // An assertion here would indicate a double-free or similar.