Ladybird+Userland: Remove use of unnecessary fd passing socket concept

Now that LibIPC is using SCM_RIGHTS properly, we can go back to only
having one socket laying around when needing to transfer fds to peers.
This commit is contained in:
Andrew Kaster
2024-04-17 18:44:39 -06:00
committed by Tim Flynn
parent cb87725ec8
commit 5e1d678bae
32 changed files with 61 additions and 236 deletions

View File

@@ -13,7 +13,7 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
WebView::ViewImplementation& view,
ReadonlySpan<ByteString> candidate_web_content_paths,
Ladybird::WebContentOptions const& web_content_options,
Optional<WebView::SocketPair> request_server_sockets)
Optional<IPC::File> request_server_socket)
{
int socket_fds[2] {};
TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, socket_fds));
@@ -21,22 +21,13 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
int ui_fd = socket_fds[0];
int wc_fd = socket_fds[1];
int fd_passing_socket_fds[2] {};
TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, fd_passing_socket_fds));
int ui_fd_passing_fd = fd_passing_socket_fds[0];
int wc_fd_passing_fd = fd_passing_socket_fds[1];
auto child_pid = TRY(Core::System::fork());
if (child_pid == 0) {
TRY(Core::System::close(ui_fd_passing_fd));
TRY(Core::System::close(ui_fd));
auto takeover_string = TRY(String::formatted("WebContent:{}", wc_fd));
TRY(Core::Environment::set("SOCKET_TAKEOVER"sv, takeover_string, Core::Environment::Overwrite::Yes));
auto webcontent_fd_passing_socket_string = TRY(String::number(wc_fd_passing_fd));
ErrorOr<void> result;
for (auto const& path : candidate_web_content_paths) {
constexpr auto callgrind_prefix_length = 3;
@@ -53,8 +44,6 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
web_content_options.command_line,
"--executable-path"sv,
web_content_options.executable_path,
"--webcontent-fd-passing-socket"sv,
webcontent_fd_passing_socket_string
};
if (web_content_options.enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::No)
arguments.remove(0, callgrind_prefix_length);
@@ -74,14 +63,11 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
arguments.append("--mach-server-name"sv);
arguments.append(server.value());
}
Vector<String> fd_strings;
if (request_server_sockets.has_value()) {
String fd_string;
if (request_server_socket.has_value()) {
arguments.append("--request-server-socket"sv);
fd_strings.append(MUST(String::number(request_server_sockets->socket.fd())));
arguments.append(fd_strings.last());
arguments.append("--request-server-fd-passing-socket"sv);
fd_strings.append(MUST(String::number(request_server_sockets->fd_passing_socket.fd())));
arguments.append(fd_strings.last());
fd_string = MUST(String::number(request_server_socket->fd()));
arguments.append(fd_string.bytes_as_string_view());
}
result = Core::System::exec(arguments[0], arguments.span(), Core::System::SearchInPath::Yes);
@@ -94,14 +80,12 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
VERIFY_NOT_REACHED();
}
TRY(Core::System::close(wc_fd_passing_fd));
TRY(Core::System::close(wc_fd));
auto socket = TRY(Core::LocalSocket::adopt_fd(ui_fd));
TRY(socket->set_blocking(true));
auto new_client = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) WebView::WebContentClient(move(socket), view)));
new_client->set_fd_passing_socket(TRY(Core::LocalSocket::adopt_fd(ui_fd_passing_fd)));
if (web_content_options.enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
dbgln();
@@ -122,22 +106,13 @@ ErrorOr<NonnullRefPtr<Client>> launch_generic_server_process(ReadonlySpan<ByteSt
int ui_fd = socket_fds[0];
int server_fd = socket_fds[1];
int fd_passing_socket_fds[2] {};
TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, fd_passing_socket_fds));
int ui_fd_passing_fd = fd_passing_socket_fds[0];
int server_fd_passing_fd = fd_passing_socket_fds[1];
auto child_pid = TRY(Core::System::fork());
if (child_pid == 0) {
TRY(Core::System::close(ui_fd));
TRY(Core::System::close(ui_fd_passing_fd));
auto takeover_string = TRY(String::formatted("{}:{}", server_name, server_fd));
TRY(Core::Environment::set("SOCKET_TAKEOVER"sv, takeover_string, Core::Environment::Overwrite::Yes));
auto fd_passing_socket_string = TRY(String::number(server_fd_passing_fd));
ErrorOr<void> result;
for (auto const& path : candidate_server_paths) {
@@ -146,8 +121,6 @@ ErrorOr<NonnullRefPtr<Client>> launch_generic_server_process(ReadonlySpan<ByteSt
auto arguments = Vector<StringView> {
path.view(),
"--fd-passing-socket"sv,
fd_passing_socket_string,
};
if (!extra_arguments.is_empty())
@@ -164,13 +137,11 @@ ErrorOr<NonnullRefPtr<Client>> launch_generic_server_process(ReadonlySpan<ByteSt
}
TRY(Core::System::close(server_fd));
TRY(Core::System::close(server_fd_passing_fd));
auto socket = TRY(Core::LocalSocket::adopt_fd(ui_fd));
TRY(socket->set_blocking(true));
auto new_client = TRY(try_make_ref_counted<Client>(move(socket)));
new_client->set_fd_passing_socket(TRY(Core::LocalSocket::adopt_fd(ui_fd_passing_fd)));
WebView::ProcessManager::the().add_process(WebView::process_type_from_name(server_name), child_pid);
@@ -184,17 +155,13 @@ ErrorOr<NonnullRefPtr<ImageDecoderClient::Client>> launch_image_decoder_process(
ErrorOr<NonnullRefPtr<Web::HTML::WebWorkerClient>> launch_web_worker_process(ReadonlySpan<ByteString> candidate_web_worker_paths, NonnullRefPtr<Protocol::RequestClient> request_client)
{
auto request_server_sockets = TRY(connect_new_request_server_client(move(request_client)));
auto socket = TRY(connect_new_request_server_client(move(request_client)));
Vector<StringView> arguments;
Vector<String> fd_strings;
String fd_string = MUST(String::number(socket.fd()));
arguments.append("--request-server-socket"sv);
fd_strings.append(MUST(String::number(request_server_sockets.socket.fd())));
arguments.append(fd_strings.last());
arguments.append("--request-server-fd-passing-socket"sv);
fd_strings.append(MUST(String::number(request_server_sockets.fd_passing_socket.fd())));
arguments.append(fd_strings.last());
arguments.append(fd_string.bytes_as_string_view());
return launch_generic_server_process<Web::HTML::WebWorkerClient>(candidate_web_worker_paths, "WebWorker"sv, move(arguments));
}
@@ -215,23 +182,21 @@ ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(Re
return launch_generic_server_process<Protocol::RequestClient>(candidate_request_server_paths, "RequestServer"sv, move(arguments));
}
ErrorOr<WebView::SocketPair> connect_new_request_server_client(Protocol::RequestClient& client)
ErrorOr<IPC::File> connect_new_request_server_client(Protocol::RequestClient& client)
{
auto new_sockets = client.send_sync_but_allow_failure<Messages::RequestServer::ConnectNewClient>();
if (!new_sockets)
auto new_socket = client.send_sync_but_allow_failure<Messages::RequestServer::ConnectNewClient>();
if (!new_socket)
return Error::from_string_literal("Failed to connect to RequestServer");
auto socket = new_sockets->take_client_socket();
auto fd_passing_socket = new_sockets->take_client_fd_passing_socket();
auto socket = new_socket->take_client_socket();
// FIXME: IPC::Files transferred over the wire are always set O_CLOEXEC during decoding.
// Perhaps we should add an option to IPC::File to allow the receiver to decide whether to
// make it O_CLOEXEC or not. Or an attribute in the .ipc file?
for (auto fd : { socket.fd(), fd_passing_socket.fd() }) {
auto fd_flags = MUST(Core::System::fcntl(fd, F_GETFD));
fd_flags &= ~FD_CLOEXEC;
MUST(Core::System::fcntl(fd, F_SETFD, fd_flags));
}
auto fd = socket.fd();
auto fd_flags = MUST(Core::System::fcntl(fd, F_GETFD));
fd_flags &= ~FD_CLOEXEC;
MUST(Core::System::fcntl(fd, F_SETFD, fd_flags));
return WebView::SocketPair { move(socket), move(fd_passing_socket) };
return socket;
}