LibIPC: Return TransportHandle directly from create_paired()

Previously, `create_paired()` returned two full Transport objects, and
callers would immediately call `from_transport()` on the remote side to
extract its underlying fd. This wasted resources: the remote
Transport's IO thread, wakeup pipes, and send queue were initialized
only to be torn down without ever sending or receiving a message.

Now `create_paired()` returns `{Transport, TransportHandle}` — the
remote side is born as a lightweight handle containing just the raw fd,
skipping all unnecessary initialization.

Also replace `release_underlying_transport_for_transfer()` (which
returned a raw int fd) with `release_for_transfer()` (which returns a
TransportHandle directly), hiding the socket implementation detail
from callers including MessagePort.
This commit is contained in:
Aliaksandr Kalenik 2026-03-14 17:34:46 +01:00 committed by Alexander Kalenik
parent e7a625185a
commit 19627bba54
11 changed files with 33 additions and 42 deletions

View file

@ -18,16 +18,10 @@ TransportHandle::TransportHandle(File file)
{
}
ErrorOr<TransportHandle> TransportHandle::from_transport(Transport& transport)
{
auto fd = TRY(transport.release_underlying_transport_for_transfer());
return TransportHandle { File::adopt_fd(fd) };
}
ErrorOr<NonnullOwnPtr<Transport>> TransportHandle::create_transport() const
{
auto socket = TRY(Core::LocalSocket::adopt_fd(m_file.take_fd()));
TRY(socket->set_blocking(true));
TRY(socket->set_blocking(false));
return make<Transport>(move(socket));
}

View file

@ -31,13 +31,11 @@ public:
TransportHandle(TransportHandle&&) = default;
TransportHandle& operator=(TransportHandle&&) = default;
static ErrorOr<TransportHandle> from_transport(Transport& transport);
ErrorOr<NonnullOwnPtr<Transport>> create_transport() const;
private:
explicit TransportHandle(File);
private:
template<typename U>
friend ErrorOr<void> encode(Encoder&, U const&);

View file

@ -12,7 +12,9 @@
#include <LibCore/Socket.h>
#include <LibCore/System.h>
#include <LibIPC/Attachment.h>
#include <LibIPC/File.h>
#include <LibIPC/Limits.h>
#include <LibIPC/TransportHandle.h>
#include <LibIPC/TransportSocket.h>
#include <LibThreading/Thread.h>
@ -31,14 +33,13 @@ ErrorOr<TransportSocket::Paired> TransportSocket::create_paired()
TRY(socket0->set_close_on_exec(true));
TRY(socket0->set_blocking(false));
auto socket1 = TRY(Core::LocalSocket::adopt_fd(fds[1]));
TRY(Core::System::set_close_on_exec(fds[1], true));
guard_fd_1.disarm();
TRY(socket1->set_close_on_exec(true));
TRY(socket1->set_blocking(false));
// Local side gets a full transport; remote side is just a handle containing the raw fd for transfer to another process.
return Paired {
make<TransportSocket>(move(socket0)),
make<TransportSocket>(move(socket1)),
TransportHandle { File::adopt_fd(fds[1]) },
};
}
@ -504,10 +505,11 @@ TransportSocket::ShouldShutdown TransportSocket::read_as_many_messages_as_possib
return m_peer_eof ? ShouldShutdown::Yes : ShouldShutdown::No;
}
ErrorOr<int> TransportSocket::release_underlying_transport_for_transfer()
ErrorOr<TransportHandle> TransportSocket::release_for_transfer()
{
stop_io_thread(IOThreadState::SendPendingMessagesAndStop);
return m_socket->release_fd();
auto fd = TRY(m_socket->release_fd());
return TransportHandle { File::adopt_fd(fd) };
}
}

View file

@ -12,7 +12,7 @@
#include <LibCore/Socket.h>
#include <LibIPC/Attachment.h>
#include <LibIPC/AutoCloseFileDescriptor.h>
#include <LibIPC/Forward.h>
#include <LibIPC/TransportHandle.h>
#include <LibThreading/ConditionVariable.h>
#include <LibThreading/Forward.h>
@ -43,7 +43,7 @@ public:
struct Paired {
NonnullOwnPtr<TransportSocket> local;
NonnullOwnPtr<TransportSocket> remote;
TransportHandle remote_handle;
};
static ErrorOr<Paired> create_paired();
@ -70,8 +70,7 @@ public:
};
ShouldShutdown read_as_many_messages_as_possible_without_blocking(Function<void(Message&&)>&&);
// Obnoxious name to make it clear that this is a dangerous operation.
ErrorOr<int> release_underlying_transport_for_transfer();
ErrorOr<TransportHandle> release_for_transfer();
private:
enum class TransferState {

View file

@ -10,8 +10,10 @@
#include <AK/ScopeGuard.h>
#include <AK/Types.h>
#include <LibCore/System.h>
#include <LibIPC/File.h>
#include <LibIPC/HandleType.h>
#include <LibIPC/Limits.h>
#include <LibIPC/TransportHandle.h>
#include <LibIPC/TransportSocketWindows.h>
#include <AK/Windows.h>
@ -31,14 +33,12 @@ ErrorOr<TransportSocketWindows::Paired> TransportSocketWindows::create_paired()
TRY(socket0->set_close_on_exec(true));
TRY(socket0->set_blocking(false));
auto socket1 = TRY(Core::LocalSocket::adopt_fd(fds[1]));
TRY(Core::System::set_close_on_exec(fds[1], true));
guard_fd_1.disarm();
TRY(socket1->set_close_on_exec(true));
TRY(socket1->set_blocking(false));
return Paired {
make<TransportSocketWindows>(move(socket0)),
make<TransportSocketWindows>(move(socket1)),
TransportHandle { File::adopt_fd(fds[1]) },
};
}
@ -272,9 +272,10 @@ TransportSocketWindows::ShouldShutdown TransportSocketWindows::read_as_many_mess
return should_shutdown;
}
ErrorOr<int> TransportSocketWindows::release_underlying_transport_for_transfer()
ErrorOr<TransportHandle> TransportSocketWindows::release_for_transfer()
{
return m_socket->release_fd();
auto fd = TRY(m_socket->release_fd());
return TransportHandle { File::adopt_fd(fd) };
}
}

View file

@ -10,6 +10,7 @@
#include <AK/Queue.h>
#include <LibCore/Socket.h>
#include <LibIPC/Attachment.h>
#include <LibIPC/TransportHandle.h>
namespace IPC {
@ -20,7 +21,7 @@ class TransportSocketWindows {
public:
struct Paired {
NonnullOwnPtr<TransportSocketWindows> local;
NonnullOwnPtr<TransportSocketWindows> remote;
TransportHandle remote_handle;
};
static ErrorOr<Paired> create_paired();
@ -46,8 +47,7 @@ public:
};
ShouldShutdown read_as_many_messages_as_possible_without_blocking(Function<void(Message&&)>&&);
// Obnoxious name to make it clear that this is a dangerous operation.
ErrorOr<int> release_underlying_transport_for_transfer();
ErrorOr<TransportHandle> release_for_transfer();
private:
ErrorOr<void> duplicate_handles(Bytes, Vector<size_t> const& handle_offsets);

View file

@ -8,13 +8,12 @@
#include <AK/ByteReader.h>
#include <AK/MemoryStream.h>
#include <LibCore/Socket.h>
#include <LibCore/System.h>
#include <LibGC/WeakHashSet.h>
#include <LibIPC/Decoder.h>
#include <LibIPC/Encoder.h>
#include <LibIPC/File.h>
#include <LibIPC/Transport.h>
#include <LibIPC/TransportHandle.h>
#include <LibWeb/Bindings/ExceptionOrUtils.h>
#include <LibWeb/Bindings/Intrinsics.h>
#include <LibWeb/Bindings/MessagePortPrototype.h>
@ -107,13 +106,12 @@ WebIDL::ExceptionOr<void> MessagePort::transfer_steps(HTML::TransferDataEncoder&
if (m_remote_port)
m_remote_port->m_has_been_shipped = true;
auto fd = MUST(m_transport->release_underlying_transport_for_transfer());
auto handle = MUST(m_transport->release_for_transfer());
m_transport.clear();
// 2. Set dataHolder.[[RemotePort]] to remotePort.
// TODO: Mach IPC
data_holder.encode(IPC_FILE_TAG);
data_holder.encode(IPC::File::adopt_fd(fd));
data_holder.encode(handle);
}
// 4. Otherwise, set dataHolder.[[RemotePort]] to null.
else {
@ -135,9 +133,8 @@ WebIDL::ExceptionOr<void> MessagePort::transfer_receiving_steps(HTML::TransferDa
// 3. If dataHolder.[[RemotePort]] is not null, then entangle dataHolder.[[RemotePort]] and value.
// (This will disentangle dataHolder.[[RemotePort]] from the original port that was transferred.)
if (auto fd_tag = data_holder.decode<u8>(); fd_tag == IPC_FILE_TAG) {
// TODO: Mach IPC
auto fd = data_holder.decode<IPC::File>();
m_transport = make<IPC::Transport>(MUST(Core::LocalSocket::adopt_fd(fd.take_fd())));
auto handle = data_holder.decode<IPC::TransportHandle>();
m_transport = MUST(handle.create_transport());
m_transport->set_up_read_hook([strong_this = GC::make_root(this)]() {
if (strong_this->m_enabled)
@ -185,7 +182,7 @@ void MessagePort::entangle_with(MessagePort& remote_port)
auto paired = MUST(IPC::Transport::create_paired());
m_transport = move(paired.local);
m_remote_port->m_transport = move(paired.remote);
m_remote_port->m_transport = MUST(paired.remote_handle.create_transport());
m_transport->set_up_read_hook([strong_this = GC::make_root(this)]() {
if (strong_this->m_enabled)

View file

@ -791,7 +791,7 @@ Messages::WebContentClient::RequestWorkerAgentResponse WebContentClient::request
auto request_server_handle = MUST(connect_new_request_server_client());
auto image_decoder_handle = MUST(connect_new_image_decoder_client());
auto worker_client = MUST(WebView::launch_web_worker_process(worker_type));
auto worker_handle = MUST(IPC::TransportHandle::from_transport(worker_client->transport()));
auto worker_handle = MUST(worker_client->transport().release_for_transfer());
return { move(worker_handle), move(request_server_handle), move(image_decoder_handle) };
}

View file

@ -17,7 +17,7 @@ template<typename WebUIType>
static ErrorOr<NonnullRefPtr<WebUIType>> create_web_ui(WebContentClient& client, String host)
{
auto paired = TRY(IPC::Transport::create_paired());
auto handle = TRY(IPC::TransportHandle::from_transport(*paired.remote));
auto handle = move(paired.remote_handle);
auto web_ui = WebUIType::create(client, move(paired.local), move(host));
client.async_connect_to_web_ui(0, move(handle));

View file

@ -58,7 +58,7 @@ Messages::ImageDecoderServer::InitTransportResponse ConnectionFromClient::init_t
ErrorOr<IPC::TransportHandle> ConnectionFromClient::connect_new_client()
{
auto paired = TRY(IPC::Transport::create_paired());
auto handle = TRY(IPC::TransportHandle::from_transport(*paired.remote));
auto handle = move(paired.remote_handle);
// Note: A ref is stored in the static s_connections map
auto client = adopt_ref(*new ConnectionFromClient(move(paired.local)));

View file

@ -141,7 +141,7 @@ Messages::RequestServer::ConnectNewClientsResponse ConnectionFromClient::connect
ErrorOr<IPC::TransportHandle> ConnectionFromClient::create_client_socket()
{
auto paired = TRY(IPC::Transport::create_paired());
auto handle = TRY(IPC::TransportHandle::from_transport(*paired.remote));
auto handle = move(paired.remote_handle);
// Note: A ref is stored in the m_connections map
auto client = adopt_ref(*new ConnectionFromClient(move(paired.local), IsPrimaryConnection::No, m_connections, m_disk_cache));