浏览代码

LibIPC+LibWeb: Add an IPC helper to transfer an IPC message buffer

This large block of code is repeated nearly verbatim in LibWeb. Move it
to a helper function that both LibIPC and LibWeb can defer to. This will
let us make changes to this method in a singular location going forward.

Note this is a bit of a regression for the MessagePort. It now suffers
from the same performance issue that IPC messages face - we prepend the
meessage size to the message buffer. This degredation is very temporary
though, as a fix is imminent, and this change makes that fix easier.
Timothy Flynn 1 年之前
父节点
当前提交
91558fa381

+ 1 - 0
Meta/gn/secondary/Userland/Libraries/LibIPC/BUILD.gn

@@ -13,6 +13,7 @@ shared_library("LibIPC") {
     "Encoder.h",
     "File.h",
     "Forward.h",
+    "Message.cpp",
     "Message.h",
     "MultiServer.h",
     "SingleServer.h",

+ 1 - 0
Userland/Libraries/LibIPC/CMakeLists.txt

@@ -2,6 +2,7 @@ set(SOURCES
     Connection.cpp
     Decoder.cpp
     Encoder.cpp
+    Message.cpp
 )
 
 serenity_lib(LibIPC ipc)

+ 3 - 45
Userland/Libraries/LibIPC/Connection.cpp

@@ -8,7 +8,6 @@
 #include <LibCore/System.h>
 #include <LibIPC/Connection.h>
 #include <LibIPC/Stub.h>
-#include <sched.h>
 #include <sys/select.h>
 
 namespace IPC {
@@ -60,50 +59,9 @@ ErrorOr<void> ConnectionBase::post_message(MessageBuffer buffer)
     if (!m_socket->is_open())
         return Error::from_string_literal("Trying to post_message during IPC shutdown");
 
-    // Prepend the message size.
-    uint32_t message_size = buffer.data.size();
-    TRY(buffer.data.try_prepend(reinterpret_cast<u8 const*>(&message_size), sizeof(message_size)));
-
-    for (auto& fd : buffer.fds) {
-        if (auto result = fd_passing_socket().send_fd(fd->value()); result.is_error()) {
-            shutdown_with_error(result.error());
-            return result;
-        }
-    }
-
-    ReadonlyBytes bytes_to_write { buffer.data.span() };
-    int writes_done = 0;
-    size_t initial_size = bytes_to_write.size();
-    while (!bytes_to_write.is_empty()) {
-        auto maybe_nwritten = m_socket->write_some(bytes_to_write);
-        writes_done++;
-        if (maybe_nwritten.is_error()) {
-            auto error = maybe_nwritten.release_error();
-            if (error.is_errno()) {
-                // FIXME: This is a hacky way to at least not crash on large messages
-                // The limit of 100 writes is arbitrary, and there to prevent indefinite spinning on the EventLoop
-                if (error.code() == EAGAIN && writes_done < 100) {
-                    sched_yield();
-                    continue;
-                }
-                shutdown_with_error(error);
-                switch (error.code()) {
-                case EPIPE:
-                    return Error::from_string_literal("IPC::Connection::post_message: Disconnected from peer");
-                case EAGAIN:
-                    return Error::from_string_literal("IPC::Connection::post_message: Peer buffer overflowed");
-                default:
-                    return Error::from_syscall("IPC::Connection::post_message write"sv, -error.code());
-                }
-            } else {
-                return error;
-            }
-        }
-
-        bytes_to_write = bytes_to_write.slice(maybe_nwritten.value());
-    }
-    if (writes_done > 1) {
-        dbgln("LibIPC::Connection FIXME Warning, needed {} writes needed to send message of size {}B, this is pretty bad, as it spins on the EventLoop", writes_done, initial_size);
+    if (auto result = buffer.transfer_message(fd_passing_socket(), *m_socket); result.is_error()) {
+        shutdown_with_error(result.error());
+        return result.release_error();
     }
 
     m_responsiveness_timer->start();

+ 62 - 0
Userland/Libraries/LibIPC/Message.cpp

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2024, Tim Flynn <trflynn89@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibCore/Socket.h>
+#include <LibIPC/Message.h>
+#include <sched.h>
+
+namespace IPC {
+
+using MessageSizeType = u32;
+
+ErrorOr<void> MessageBuffer::transfer_message(Core::LocalSocket& fd_passing_socket, Core::LocalSocket& data_socket)
+{
+    MessageSizeType message_size = data.size();
+    TRY(data.try_prepend(reinterpret_cast<u8 const*>(&message_size), sizeof(message_size)));
+
+    for (auto const& fd : fds)
+        TRY(fd_passing_socket.send_fd(fd->value()));
+
+    ReadonlyBytes bytes_to_write { data.span() };
+    size_t writes_done = 0;
+
+    while (!bytes_to_write.is_empty()) {
+        auto maybe_nwritten = data_socket.write_some(bytes_to_write);
+        ++writes_done;
+
+        if (maybe_nwritten.is_error()) {
+            if (auto error = maybe_nwritten.release_error(); error.is_errno()) {
+                // FIXME: This is a hacky way to at least not crash on large messages
+                // The limit of 100 writes is arbitrary, and there to prevent indefinite spinning on the EventLoop
+                if (error.code() == EAGAIN && writes_done < 100) {
+                    sched_yield();
+                    continue;
+                }
+
+                switch (error.code()) {
+                case EPIPE:
+                    return Error::from_string_literal("IPC::transfer_message: Disconnected from peer");
+                case EAGAIN:
+                    return Error::from_string_literal("IPC::transfer_message: Peer buffer overflowed");
+                default:
+                    return Error::from_syscall("IPC::transfer_message write"sv, -error.code());
+                }
+            } else {
+                return error;
+            }
+        }
+
+        bytes_to_write = bytes_to_write.slice(maybe_nwritten.value());
+    }
+
+    if (writes_done > 1) {
+        dbgln("LibIPC::transfer_message FIXME Warning, needed {} writes needed to send message of size {}B, this is pretty bad, as it spins on the EventLoop", writes_done, data.size());
+    }
+
+    return {};
+}
+
+}

+ 4 - 0
Userland/Libraries/LibIPC/Message.h

@@ -10,6 +10,8 @@
 #include <AK/Error.h>
 #include <AK/RefCounted.h>
 #include <AK/RefPtr.h>
+#include <AK/Vector.h>
+#include <LibCore/Forward.h>
 #include <unistd.h>
 
 namespace IPC {
@@ -34,6 +36,8 @@ private:
 };
 
 struct MessageBuffer {
+    ErrorOr<void> transfer_message(Core::LocalSocket& fd_passing_socket, Core::LocalSocket& data_socket);
+
     Vector<u8, 1024> data;
     Vector<NonnullRefPtr<AutoCloseFileDescriptor>, 1> fds;
 };

+ 1 - 46
Userland/Libraries/LibWeb/HTML/MessagePort.cpp

@@ -253,54 +253,9 @@ ErrorOr<void> MessagePort::send_message_on_socket(SerializedTransferRecord const
 {
     IPC::MessageBuffer buffer;
     IPC::Encoder encoder(buffer);
-    MUST(encoder.encode<u32>(0)); // placeholder for total size
     MUST(encoder.encode(serialize_with_transfer_result));
 
-    u32 buffer_size = buffer.data.size() - sizeof(u32); // size of *payload*
-    buffer.data[0] = buffer_size & 0xFF;
-    buffer.data[1] = (buffer_size >> 8) & 0xFF;
-    buffer.data[2] = (buffer_size >> 16) & 0xFF;
-    buffer.data[3] = (buffer_size >> 24) & 0xFF;
-
-    for (auto& fd : buffer.fds) {
-        if (auto result = m_fd_passing_socket->send_fd(fd->value()); result.is_error()) {
-            return Error::from_string_view("Can't send fd"sv);
-        }
-    }
-
-    ReadonlyBytes bytes_to_write { buffer.data.span() };
-    int writes_done = 0;
-    size_t initial_size = bytes_to_write.size();
-    while (!bytes_to_write.is_empty()) {
-        auto maybe_nwritten = m_socket->write_some(bytes_to_write);
-        writes_done++;
-        if (maybe_nwritten.is_error()) {
-            auto error = maybe_nwritten.release_error();
-            if (error.is_errno()) {
-                // FIXME: This is a hacky way to at least not crash on large messages
-                // The limit of 100 writes is arbitrary, and there to prevent indefinite spinning on the EventLoop
-                if (error.code() == EAGAIN && writes_done < 100) {
-                    sched_yield();
-                    continue;
-                }
-                switch (error.code()) {
-                case EPIPE:
-                    return Error::from_string_literal("IPC::Connection::post_message: Disconnected from peer");
-                case EAGAIN:
-                    return Error::from_string_literal("IPC::Connection::post_message: Peer buffer overflowed");
-                default:
-                    return Error::from_syscall("IPC::Connection::post_message write"sv, -error.code());
-                }
-            } else {
-                return error;
-            }
-        }
-
-        bytes_to_write = bytes_to_write.slice(maybe_nwritten.value());
-    }
-    if (writes_done > 1) {
-        dbgln("LibIPC::Connection FIXME Warning, needed {} writes needed to send message of size {}B, this is pretty bad, as it spins on the EventLoop", writes_done, initial_size);
-    }
+    TRY(buffer.transfer_message(*m_fd_passing_socket, *m_socket));
     return {};
 }