Repository: microsoft/terminal
File: src/cascadia/TerminalConnection/CTerminalHandoff.cpp (C++)

Code before:

// Routine Description:
// - Receives the terminal handoff via COM from the other process,
//   duplicates handles as COM will free those given on the way out,
//   then fires off an event notifying the rest of the terminal that
//   a connection is on its way in.
// Arguments:
// - in - PTY input handle that we will read from
// - out - PTY output handle that we will write to
// - signal - PTY signal handle for out of band messaging
// - ref - Client reference handle for console session so it stays alive until we let go
// - server - PTY process handle to track for lifetime/cleanup
// - client - Process handle to client so we can track its lifetime and exit appropriately
// Return Value:
// - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle`
//   error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error
//   from the registered handler event function.
HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept
{
    // Stash a local copy of _pfnHandoff before we stop listening.
    auto localPfnHandoff = _pfnHandoff;

    // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
    // COM does not automatically clean that up for us. We must do it.
    s_StopListening();

    // Report an error if no one registered a handoff function before calling this.
    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);

    // Duplicate the handles from what we received.
    // The contract with COM specifies that any HANDLEs we receive from the caller belong
    // to the caller and will be freed when we leave the scope of this method.
    // Making our own duplicate copy ensures they hang around in our lifetime.
    RETURN_IF_FAILED(_duplicateHandle(in, in));
    RETURN_IF_FAILED(_duplicateHandle(out, out));
    RETURN_IF_FAILED(_duplicateHandle(signal, signal));
    RETURN_IF_FAILED(_duplicateHandle(ref, ref));
    RETURN_IF_FAILED(_duplicateHandle(server, server));
    RETURN_IF_FAILED(_duplicateHandle(client, client));

    // Call registered handler from when we started listening.
    return localPfnHandoff(in, out, signal, ref, server, client);
}


Review comment (suggestion):
I wonder if there is a race here between the monarch turning on a listener at the same time on another thread as a previous request is being served out of the mta Com pool and turning it off. Maybe we should make methods on this class like startlistening, stoplistening, and/or this establishptyhandoff one use a synchronization primitive so multiple things can't happen to it at once... 

Code after:
// - S_OK or Win32 error from `::DuplicateHandle`
static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
{
    RETURN_IF_WIN32_BOOL_FALSE(::DuplicateHandle(GetCurrentProcess(), in, GetCurrentProcess(), &out, 0, FALSE, DUPLICATE_SAME_ACCESS));
    return S_OK;
}

// Routine Description:
// - Receives the terminal handoff via COM from the other process,
//   duplicates handles as COM will free those given on the way out,
//   then fires off an event notifying the rest of the terminal that
//   a connection is on its way in.
// Arguments:
// - in - PTY input handle that we will read from
// - out - PTY output handle that we will write to
// - signal - PTY signal handle for out of band messaging
// - ref - Client reference handle for console session so it stays alive until we let go
// - server - PTY process handle to track for lifetime/cleanup
// - client - Process handle to client so we can track its lifetime and exit appropriately
// Return Value:
// - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle`
//   error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error
//   from the registered handler event function.
HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client)
{
    // Stash a local copy of _pfnHandoff before we stop listening.
    auto localPfnHandoff = _pfnHandoff;

    // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
    // COM does not automatically clean that up for us. We must do it.
    s_StopListening();

    std::unique_lock lock{ _mtx };

    // Report an error if no one registered a handoff function before calling this.
    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);

    // Duplicate the handles from what we received.
    // The contract with COM specifies that any HANDLEs we receive from the caller belong
    // to the caller and will be freed when we leave the scope of this method.
    // Making our own duplicate copy ensures they hang around in our lifetime.
    RETURN_IF_FAILED(_duplicateHandle(in, in));
    RETURN_IF_FAILED(_duplicateHandle(out, out));
    RETURN_IF_FAILED(_duplicateHandle(signal, signal));
    RETURN_IF_FAILED(_duplicateHandle(ref, ref));
    RETURN_IF_FAILED(_duplicateHandle(server, server));
    RETURN_IF_FAILED(_duplicateHandle(client, client));

    // Call registered handler from when we started listening.
    return localPfnHandoff(in, out, signal, ref, server, client);
}