Repository: LadybirdBrowser/ladybird
File: Libraries/LibIPC/Connection.cpp (C++)

Code before:

void ConnectionBase::wait_for_transport_to_become_readable()
{
    m_transport->wait_until_readable();
}

ErrorOr<void> ConnectionBase::drain_messages_from_peer()
{
    auto schedule_shutdown = m_transport->read_as_many_messages_as_possible_without_blocking([&](auto&& raw_message) {
        if (auto message = try_parse_message(raw_message.bytes, raw_message.fds)) {
            m_unprocessed_messages.append(message.release_nonnull());
        } else {
            dbgln("Failed to parse IPC message {:hex-dump}", raw_message.bytes);
            VERIFY_NOT_REACHED();
        }
    });

    if (!m_unprocessed_messages.is_empty()) {
        m_responsiveness_timer->stop();
        did_become_responsive();
        deferred_invoke([this] {
            handle_messages();
        });
    }

    if (schedule_shutdown == TransportSocket::ShouldShutdown::Yes) {
        deferred_invoke([this] {
            shutdown();
        });
        return Error::from_string_literal("IPC connection EOF");
    }

    return {};
}

OwnPtr<IPC::Message> ConnectionBase::wait_for_specific_endpoint_message_impl(u32 endpoint_magic, int message_id)
{
    for (;;) {
        // Double check we don't already have the event waiting for us.
        // Otherwise we might end up blocked for a while for no reason.
        for (size_t i = 0; i < m_unprocessed_messages.size(); ++i) {
            auto& message = m_unprocessed_messages[i];
            if (message->endpoint_magic() != endpoint_magic)
                continue;
            if (message->message_id() == message_id)
                return m_unprocessed_messages.take(i);
        }

        if (!is_open())
            break;


Review comment (bug):
Sorry for not actually leaving this comment.

When I looked at this a while back I had the question: Is this really the right behavior?

From looking at this, it seems like we will end up dropping however many unprocessed messages are in the queue when doing this shutdown. Given your example uses cookies, which involve a lot of bidirecitonal IPC, is the 'true' solution somewhere closer to checking for IPC 'shutdown requested' before sending more messages across? If there are actual important IPC messages in the unprocessed message queue that we don't want to drop, this patch would make it a lot harder to debug that happening. 

Code after:
                dbgln("IPC::ConnectionBase::handle_messages: {}", post_result.error());
        }
    }
}

void ConnectionBase::wait_for_transport_to_become_readable()
{
    m_transport->wait_until_readable();
}

ErrorOr<void> ConnectionBase::drain_messages_from_peer()
{
    auto schedule_shutdown = m_transport->read_as_many_messages_as_possible_without_blocking([&](auto&& raw_message) {
        if (auto message = try_parse_message(raw_message.bytes, raw_message.fds)) {
            m_unprocessed_messages.append(message.release_nonnull());
        } else {
            dbgln("Failed to parse IPC message {:hex-dump}", raw_message.bytes);
            VERIFY_NOT_REACHED();
        }
    });

    if (!m_unprocessed_messages.is_empty()) {
        deferred_invoke([this] {
            handle_messages();
        });
    }

    if (schedule_shutdown == Transport::ShouldShutdown::Yes) {
        deferred_invoke([this] {
            shutdown();
        });
        return Error::from_string_literal("IPC connection EOF");
    }

    return {};
}

OwnPtr<IPC::Message> ConnectionBase::wait_for_specific_endpoint_message_impl(u32 endpoint_magic, int message_id)
{
    for (;;) {
        // Double check we don't already have the event waiting for us.
        // Otherwise we might end up blocked for a while for no reason.
        for (size_t i = 0; i < m_unprocessed_messages.size(); ++i) {
            auto& message = m_unprocessed_messages[i];
            if (message->endpoint_magic() != endpoint_magic)
                continue;
            if (message->message_id() == message_id)
                return m_unprocessed_messages.take(i);
        }

        if (!is_open())