mirror of
https://github.com/LadybirdBrowser/ladybird
synced 2026-04-25 17:25:08 +02:00
Everywhere: Simplify Mach bootstrap transport handshake
Previously, the bootstrap handshake used a two-state machine (WaitingForPorts / WaitingForReplyPort) to handle a race: the parent registering transport ports and the child sending a bootstrap request could arrive in either order, so whichever came first stored its half and the second completed the handshake. Eliminate the race by holding a mutex across spawn() and register_child_transport(). Since the child cannot send a bootstrap request before it exists, and the lock isn't released until its transport is registered, handle_bootstrap_request() is guaranteed to find the entry. This reduces the pending map to a simple pid-to-ports lookup and collapses the two-variant state into two straightforward branches: known child, or on-demand (non-child) caller like WebDriver.
This commit is contained in:
committed by
Alexander Kalenik
parent
2b9cce8f62
commit
e47f4cf90f
Notes:
github-actions[bot]
2026-03-24 18:53:25 +00:00
Author: https://github.com/kalenikaliaksandr Commit: https://github.com/LadybirdBrowser/ladybird/commit/e47f4cf90f0 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/8599
@@ -13,7 +13,6 @@
|
||||
#include <LibIPC/TransportBootstrapMach.h>
|
||||
|
||||
#include <mach/mach.h>
|
||||
#include <sys/sysctl.h>
|
||||
|
||||
namespace IPC {
|
||||
|
||||
@@ -84,26 +83,6 @@ void TransportBootstrapMachServer::send_transport_ports_to_child(Core::MachPort
|
||||
VERIFY(ret == KERN_SUCCESS);
|
||||
}
|
||||
|
||||
static bool process_is_child_of_us(pid_t pid)
|
||||
{
|
||||
int mib[4] = {};
|
||||
struct kinfo_proc info = {};
|
||||
size_t size = sizeof(info);
|
||||
|
||||
mib[0] = CTL_KERN;
|
||||
mib[1] = KERN_PROC;
|
||||
mib[2] = KERN_PROC_PID;
|
||||
mib[3] = pid;
|
||||
|
||||
if (sysctl(mib, sizeof(mib) / sizeof(*mib), &info, &size, nullptr, 0) < 0)
|
||||
return false;
|
||||
|
||||
if (size == 0)
|
||||
return false;
|
||||
|
||||
return info.kp_eproc.e_ppid == Core::System::getpid();
|
||||
}
|
||||
|
||||
ErrorOr<TransportBootstrapMachPorts> TransportBootstrapMachServer::create_on_demand_local_transport(Core::MachPort reply_port)
|
||||
{
|
||||
auto local_receive_right = TRY(Core::MachPort::create_with_right(Core::MachPort::PortRight::Receive));
|
||||
@@ -123,52 +102,26 @@ ErrorOr<TransportBootstrapMachPorts> TransportBootstrapMachServer::create_on_dem
|
||||
};
|
||||
}
|
||||
|
||||
void TransportBootstrapMachServer::register_pending_transport(pid_t pid, TransportBootstrapMachPorts ports)
|
||||
void TransportBootstrapMachServer::register_child_transport(pid_t pid, TransportBootstrapMachPorts ports)
|
||||
{
|
||||
Optional<PendingBootstrapState> pending;
|
||||
VERIFY(!m_child_transports.contains(pid));
|
||||
m_child_transports.set(pid, move(ports));
|
||||
}
|
||||
|
||||
ErrorOr<TransportBootstrapMachServer::BootstrapRequestResult> TransportBootstrapMachServer::handle_bootstrap_request(pid_t pid, Core::MachPort reply_port)
|
||||
{
|
||||
Optional<TransportBootstrapMachPorts> child_transport;
|
||||
{
|
||||
Threading::MutexLocker locker(m_pending_bootstrap_mutex);
|
||||
pending = m_pending_bootstrap.take(pid);
|
||||
if (!pending.has_value()) {
|
||||
m_pending_bootstrap.set(pid, WaitingForPorts { move(ports) });
|
||||
return;
|
||||
}
|
||||
Threading::MutexLocker locker(m_child_registration_mutex);
|
||||
child_transport = m_child_transports.take(pid);
|
||||
}
|
||||
|
||||
pending.release_value().visit(
|
||||
[&](WaitingForPorts&) {
|
||||
VERIFY_NOT_REACHED();
|
||||
},
|
||||
[&](WaitingForReplyPort& waiting) {
|
||||
send_transport_ports_to_child(move(waiting.reply_port), move(ports));
|
||||
});
|
||||
}
|
||||
|
||||
ErrorOr<TransportBootstrapMachServer::RegisterReplyPortResult> TransportBootstrapMachServer::register_reply_port(pid_t pid, Core::MachPort reply_port)
|
||||
{
|
||||
Optional<PendingBootstrapState> pending;
|
||||
{
|
||||
Threading::MutexLocker locker(m_pending_bootstrap_mutex);
|
||||
pending = m_pending_bootstrap.take(pid);
|
||||
if (!pending.has_value()) {
|
||||
if (process_is_child_of_us(pid)) {
|
||||
m_pending_bootstrap.set(pid, WaitingForReplyPort { move(reply_port) });
|
||||
return WaitingForChildTransport {};
|
||||
}
|
||||
}
|
||||
if (child_transport.has_value()) {
|
||||
send_transport_ports_to_child(move(reply_port), child_transport.release_value());
|
||||
return ChildTransportHandled {};
|
||||
}
|
||||
|
||||
if (!pending.has_value())
|
||||
return OnDemandTransport { .ports = TRY(create_on_demand_local_transport(move(reply_port))) };
|
||||
|
||||
return pending.release_value().visit(
|
||||
[&](WaitingForPorts& waiting) -> ErrorOr<RegisterReplyPortResult> {
|
||||
send_transport_ports_to_child(move(reply_port), move(waiting.ports));
|
||||
return WaitingForChildTransport {};
|
||||
},
|
||||
[&](WaitingForReplyPort&) -> ErrorOr<RegisterReplyPortResult> {
|
||||
VERIFY_NOT_REACHED();
|
||||
});
|
||||
return OnDemandTransport { .ports = TRY(create_on_demand_local_transport(move(reply_port))) };
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -34,30 +34,27 @@ class TransportBootstrapMachServer {
|
||||
public:
|
||||
TransportBootstrapMachServer() = default;
|
||||
|
||||
struct WaitingForChildTransport {
|
||||
struct ChildTransportHandled {
|
||||
};
|
||||
struct OnDemandTransport {
|
||||
TransportBootstrapMachPorts ports;
|
||||
};
|
||||
using RegisterReplyPortResult = Variant<WaitingForChildTransport, OnDemandTransport>;
|
||||
using BootstrapRequestResult = Variant<ChildTransportHandled, OnDemandTransport>;
|
||||
|
||||
void register_pending_transport(pid_t, TransportBootstrapMachPorts);
|
||||
ErrorOr<RegisterReplyPortResult> register_reply_port(pid_t, Core::MachPort reply_port);
|
||||
// Hold this lock across process spawn and child transport registration so a
|
||||
// child bootstrap request cannot observe an unregistered pid.
|
||||
Threading::Mutex& child_registration_lock() { return m_child_registration_mutex; }
|
||||
|
||||
// Must be called while holding child_registration_lock().
|
||||
void register_child_transport(pid_t, TransportBootstrapMachPorts);
|
||||
ErrorOr<BootstrapRequestResult> handle_bootstrap_request(pid_t, Core::MachPort reply_port);
|
||||
|
||||
private:
|
||||
struct WaitingForPorts {
|
||||
TransportBootstrapMachPorts ports;
|
||||
};
|
||||
struct WaitingForReplyPort {
|
||||
Core::MachPort reply_port;
|
||||
};
|
||||
using PendingBootstrapState = Variant<WaitingForPorts, WaitingForReplyPort>;
|
||||
|
||||
static void send_transport_ports_to_child(Core::MachPort reply_port, TransportBootstrapMachPorts ports);
|
||||
static ErrorOr<TransportBootstrapMachPorts> create_on_demand_local_transport(Core::MachPort reply_port);
|
||||
|
||||
Threading::Mutex m_pending_bootstrap_mutex;
|
||||
HashMap<pid_t, PendingBootstrapState> m_pending_bootstrap;
|
||||
Threading::Mutex m_child_registration_mutex;
|
||||
HashMap<pid_t, TransportBootstrapMachPorts> m_child_transports;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
@@ -106,16 +106,11 @@ ErrorOr<void> Application::initialize(Main::Arguments const& arguments)
|
||||
m_mach_port_server = make<MachPortServer>(mach_server_name_for_process("Ladybird"sv, Core::System::getpid()));
|
||||
set_mach_server_name(m_mach_port_server->server_port_name());
|
||||
|
||||
m_mach_port_server->on_receive_child_mach_port = [this](MachPortServer::ChildMachPortRegistration registration) {
|
||||
set_process_mach_port(registration.pid, move(registration.child_port));
|
||||
auto registration_result = m_transport_bootstrap_server.register_reply_port(registration.pid, move(registration.reply_port));
|
||||
if (registration_result.is_error()) {
|
||||
dbgln("Failed to bootstrap Mach transport for pid {}: {}", registration.pid, registration_result.error());
|
||||
return;
|
||||
}
|
||||
|
||||
registration_result.release_value().visit(
|
||||
[](IPC::TransportBootstrapMachServer::WaitingForChildTransport) {
|
||||
m_mach_port_server->on_bootstrap_request = [this](MachPortServer::BootstrapRequest request) {
|
||||
set_process_mach_port(request.pid, move(request.task_port));
|
||||
auto result = MUST(m_transport_bootstrap_server.handle_bootstrap_request(request.pid, move(request.reply_port)));
|
||||
result.visit(
|
||||
[](IPC::TransportBootstrapMachServer::ChildTransportHandled) {
|
||||
},
|
||||
[this](IPC::TransportBootstrapMachServer::OnDemandTransport& transport) {
|
||||
if (!m_on_browser_process_transport)
|
||||
|
||||
@@ -74,14 +74,14 @@ void MachPortServer::thread_loop()
|
||||
VERIFY(task_port_message.body.msgh_descriptor_count == 1);
|
||||
VERIFY(task_port_message.port_descriptor.type == MACH_MSG_PORT_DESCRIPTOR);
|
||||
auto pid = static_cast<pid_t>(task_port_message.trailer.msgh_audit.val[5]);
|
||||
auto child_port = Core::MachPort::adopt_right(task_port_message.port_descriptor.name, Core::MachPort::PortRight::Send);
|
||||
auto task_port = Core::MachPort::adopt_right(task_port_message.port_descriptor.name, Core::MachPort::PortRight::Send);
|
||||
|
||||
// Extract reply port from the message header (kernel swaps local/remote on receive)
|
||||
auto reply_port = Core::MachPort::adopt_right(message.header.msgh_remote_port, Core::MachPort::PortRight::SendOnce);
|
||||
|
||||
dbgln_if(MACH_PORT_DEBUG, "Received child port {:x} from pid {} (reply port {:x})", child_port.port(), pid, reply_port.port());
|
||||
if (on_receive_child_mach_port)
|
||||
on_receive_child_mach_port({ pid, move(child_port), move(reply_port) });
|
||||
dbgln_if(MACH_PORT_DEBUG, "Received bootstrap request from pid {} (task port {:x}, reply port {:x})", pid, task_port.port(), reply_port.port());
|
||||
VERIFY(on_bootstrap_request);
|
||||
on_bootstrap_request({ pid, move(task_port), move(reply_port) });
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -30,12 +30,12 @@ public:
|
||||
|
||||
bool is_initialized();
|
||||
|
||||
struct ChildMachPortRegistration {
|
||||
struct BootstrapRequest {
|
||||
pid_t pid { -1 };
|
||||
Core::MachPort child_port;
|
||||
Core::MachPort task_port;
|
||||
Core::MachPort reply_port;
|
||||
};
|
||||
Function<void(ChildMachPortRegistration)> on_receive_child_mach_port;
|
||||
Function<void(BootstrapRequest)> on_bootstrap_request;
|
||||
|
||||
ByteString const& server_port_name() const { return m_server_port_name; }
|
||||
|
||||
|
||||
@@ -69,9 +69,10 @@ ErrorOr<Process::ProcessAndIPCTransport> Process::spawn_and_connect_to_process(C
|
||||
auto port_b_recv = TRY(Core::MachPort::create_with_right(Core::MachPort::PortRight::Receive));
|
||||
auto port_b_send = TRY(port_b_recv.insert_right(Core::MachPort::MessageRight::MakeSend));
|
||||
|
||||
Threading::MutexLocker child_registration_locker(Application::transport_bootstrap_server().child_registration_lock());
|
||||
auto process = TRY(Core::Process::spawn(spawn_options));
|
||||
|
||||
Application::transport_bootstrap_server().register_pending_transport(process.pid(), IPC::TransportBootstrapMachPorts { move(port_b_recv), move(port_a_send) });
|
||||
Application::transport_bootstrap_server().register_child_transport(process.pid(), IPC::TransportBootstrapMachPorts { move(port_b_recv), move(port_a_send) });
|
||||
|
||||
auto transport = make<IPC::Transport>(move(port_a_recv), move(port_b_send));
|
||||
#else
|
||||
|
||||
@@ -275,19 +275,19 @@ ErrorOr<void> Session::create_server(NonnullRefPtr<ServerPromise> promise)
|
||||
if (!m_web_content_mach_port_server->is_initialized())
|
||||
return Error::from_string_literal("Failed to initialize Mach port server for WebDriver");
|
||||
|
||||
m_web_content_mach_port_server->on_receive_child_mach_port = [this, promise](auto registration) {
|
||||
auto registration_result = m_transport_bootstrap_server.register_reply_port(registration.pid, move(registration.reply_port));
|
||||
if (registration_result.is_error()) {
|
||||
m_web_content_mach_port_server->on_bootstrap_request = [this, promise](auto request) {
|
||||
auto result = m_transport_bootstrap_server.handle_bootstrap_request(request.pid, move(request.reply_port));
|
||||
if (result.is_error()) {
|
||||
auto event_loop = m_event_loop->take();
|
||||
VERIFY(event_loop);
|
||||
event_loop->deferred_invoke([promise, error = registration_result.release_error()]() mutable {
|
||||
event_loop->deferred_invoke([promise, error = result.release_error()]() mutable {
|
||||
promise->resolve(move(error));
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
registration_result.release_value().visit(
|
||||
[](IPC::TransportBootstrapMachServer::WaitingForChildTransport) {
|
||||
result.release_value().visit(
|
||||
[](IPC::TransportBootstrapMachServer::ChildTransportHandled) {
|
||||
VERIFY_NOT_REACHED();
|
||||
},
|
||||
[this, promise](IPC::TransportBootstrapMachServer::OnDemandTransport& transport) {
|
||||
|
||||
Reference in New Issue
Block a user