From 2e1bbcb0faeae92d7595b8e0b022a8cdcecca07e Mon Sep 17 00:00:00 2001 From: sin-ack Date: Fri, 14 Jan 2022 13:12:49 +0000 Subject: [PATCH] LibCore+LibIPC+Everywhere: Return Stream::LocalSocket from LocalServer This change unfortunately cannot be atomically made without a single commit changing everything. Most of the important changes are in LibIPC/Connection.cpp, LibIPC/ServerConnection.cpp and LibCore/LocalServer.cpp. The notable changes are: - IPCCompiler now generates the decode and decode_message functions such that they take a Core::Stream::LocalSocket instead of the socket fd. - IPC::Decoder now uses the receive_fd method of LocalSocket instead of doing system calls directly on the fd. - IPC::ConnectionBase and related classes now use the Stream API functions. - IPC::ServerConnection no longer constructs the socket itself; instead, a convenience macro, IPC_CLIENT_CONNECTION, is used in place of C_OBJECT and will generate a static try_create factory function for the ServerConnection subclass. The subclass is now responsible for passing the socket constructed in this function to its ServerConnection base; the socket is passed as the first argument to the constructor (as a NonnullOwnPtr) before any other arguments. - The functionality regarding taking over sockets from SystemServer has been moved to LibIPC/SystemServerTakeover.cpp. The Core::LocalSocket implementation of this functionality hasn't been deleted due to my intention of removing this class in the near future and to reduce noise on this (already quite noisy) PR. --- .../Tools/CodeGenerators/IPCCompiler/main.cpp | 8 +- Tests/LibCore/TestLibCoreStream.cpp | 17 ++-- Userland/Applets/Audio/main.cpp | 7 +- .../Applications/ImageViewer/ViewWidget.cpp | 2 +- .../Applications/Piano/AudioPlayerLoop.cpp | 2 +- Userland/Applications/PixelPaint/Image.cpp | 2 +- Userland/DevTools/HackStudio/LanguageClient.h | 6 +- .../LanguageClients/ServerConnections.h | 26 +++--- .../LanguageServers/ClientConnection.cpp | 2 +- .../LanguageServers/ClientConnection.h | 2 +- .../LanguageServers/Cpp/ClientConnection.h | 2 +- .../LanguageServers/Shell/ClientConnection.h | 2 +- .../Inspector/InspectorServerClient.h | 6 +- Userland/DevTools/Inspector/RemoteProcess.cpp | 2 +- .../Libraries/LibAudio/ClientConnection.cpp | 4 +- .../Libraries/LibAudio/ClientConnection.h | 4 +- Userland/Libraries/LibConfig/Client.cpp | 2 +- Userland/Libraries/LibConfig/Client.h | 6 +- Userland/Libraries/LibCore/LocalServer.cpp | 17 ++-- Userland/Libraries/LibCore/LocalServer.h | 5 +- Userland/Libraries/LibDesktop/Launcher.cpp | 8 +- .../LibFileSystemAccessClient/Client.cpp | 2 +- .../LibFileSystemAccessClient/Client.h | 6 +- Userland/Libraries/LibGUI/Clipboard.cpp | 10 +-- Userland/Libraries/LibGUI/Notification.cpp | 8 +- .../LibGUI/WindowManagerServerConnection.cpp | 4 +- .../LibGUI/WindowManagerServerConnection.h | 7 +- .../LibGUI/WindowServerConnection.cpp | 8 +- .../Libraries/LibGUI/WindowServerConnection.h | 4 +- Userland/Libraries/LibIPC/CMakeLists.txt | 1 + Userland/Libraries/LibIPC/ClientConnection.h | 4 +- Userland/Libraries/LibIPC/Connection.cpp | 82 ++++++++++--------- Userland/Libraries/LibIPC/Connection.h | 17 ++-- Userland/Libraries/LibIPC/Decoder.cpp | 7 +- Userland/Libraries/LibIPC/Decoder.h | 7 +- Userland/Libraries/LibIPC/ServerConnection.h | 27 +++--- Userland/Libraries/LibIPC/SingleServer.h | 4 +- .../Libraries/LibIPC/SystemServerTakeover.cpp | 65 +++++++++++++++ .../Libraries/LibIPC/SystemServerTakeover.h | 12 +++ .../LibImageDecoderClient/Client.cpp | 4 +- .../Libraries/LibImageDecoderClient/Client.h | 4 +- .../Libraries/LibProtocol/RequestClient.cpp | 4 +- .../Libraries/LibProtocol/RequestClient.h | 4 +- .../Libraries/LibProtocol/WebSocketClient.cpp | 4 +- .../Libraries/LibProtocol/WebSocketClient.h | 4 +- Userland/Libraries/LibSQL/SQLClient.h | 6 +- Userland/Libraries/LibWeb/HTML/WebSocket.cpp | 14 +++- Userland/Libraries/LibWeb/HTML/WebSocket.h | 6 +- Userland/Libraries/LibWeb/ImageDecoding.cpp | 2 +- .../LibWeb/Loader/ResourceLoader.cpp | 15 +++- .../Libraries/LibWeb/Loader/ResourceLoader.h | 6 +- .../Libraries/LibWeb/OutOfProcessWebView.cpp | 2 +- .../Libraries/LibWeb/WebContentClient.cpp | 4 +- Userland/Libraries/LibWeb/WebContentClient.h | 4 +- .../Services/AudioServer/ClientConnection.cpp | 2 +- .../Services/AudioServer/ClientConnection.h | 2 +- Userland/Services/AudioServer/main.cpp | 2 +- .../Services/Clipboard/ClientConnection.cpp | 2 +- .../Services/Clipboard/ClientConnection.h | 2 +- .../ConfigServer/ClientConnection.cpp | 2 +- .../Services/ConfigServer/ClientConnection.h | 2 +- .../ClientConnection.cpp | 4 +- .../FileSystemAccessServer/ClientConnection.h | 2 +- .../ImageDecoder/ClientConnection.cpp | 2 +- .../Services/ImageDecoder/ClientConnection.h | 2 +- .../InspectorServer/ClientConnection.cpp | 2 +- .../InspectorServer/ClientConnection.h | 2 +- .../InspectorServer/InspectableProcess.cpp | 46 ++++++----- .../InspectorServer/InspectableProcess.h | 6 +- Userland/Services/InspectorServer/main.cpp | 2 +- .../LaunchServer/ClientConnection.cpp | 2 +- .../Services/LaunchServer/ClientConnection.h | 2 +- .../LookupServer/ClientConnection.cpp | 2 +- .../Services/LookupServer/ClientConnection.h | 2 +- .../NotificationServer/ClientConnection.cpp | 2 +- .../NotificationServer/ClientConnection.h | 2 +- .../RequestServer/ClientConnection.cpp | 2 +- .../Services/RequestServer/ClientConnection.h | 2 +- .../Services/SQLServer/ClientConnection.cpp | 2 +- .../Services/SQLServer/ClientConnection.h | 2 +- .../SpiceAgent/ClipboardServerConnection.h | 6 +- .../Services/WebContent/ClientConnection.cpp | 2 +- .../Services/WebContent/ClientConnection.h | 2 +- .../Services/WebSocket/ClientConnection.cpp | 2 +- .../Services/WebSocket/ClientConnection.h | 2 +- .../WindowServer/ClientConnection.cpp | 2 +- .../Services/WindowServer/ClientConnection.h | 2 +- .../WindowServer/WMClientConnection.cpp | 2 +- .../WindowServer/WMClientConnection.h | 2 +- Userland/Utilities/aplay.cpp | 2 +- Userland/Utilities/asctl.cpp | 2 +- Userland/Utilities/pro.cpp | 2 +- Userland/Utilities/sql.cpp | 2 +- Userland/Utilities/telws.cpp | 8 +- 94 files changed, 378 insertions(+), 252 deletions(-) create mode 100644 Userland/Libraries/LibIPC/SystemServerTakeover.cpp create mode 100644 Userland/Libraries/LibIPC/SystemServerTakeover.h diff --git a/Meta/Lagom/Tools/CodeGenerators/IPCCompiler/main.cpp b/Meta/Lagom/Tools/CodeGenerators/IPCCompiler/main.cpp index 069507f49b2..78de3ca3ce8 100644 --- a/Meta/Lagom/Tools/CodeGenerators/IPCCompiler/main.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/IPCCompiler/main.cpp @@ -368,9 +368,9 @@ public: static i32 static_message_id() { return (int)MessageID::@message.pascal_name@; } virtual const char* message_name() const override { return "@endpoint.name@::@message.pascal_name@"; } - static OwnPtr<@message.pascal_name@> decode(InputMemoryStream& stream, [[maybe_unused]] int sockfd) + static OwnPtr<@message.pascal_name@> decode(InputMemoryStream& stream, Core::Stream::LocalSocket& socket) { - IPC::Decoder decoder { stream, sockfd }; + IPC::Decoder decoder { stream, socket }; )~~~"); for (auto& parameter : parameters) { @@ -632,7 +632,7 @@ public: static u32 static_magic() { return @endpoint.magic@; } - static OwnPtr decode_message(ReadonlyBytes buffer, [[maybe_unused]] int sockfd) + static OwnPtr decode_message(ReadonlyBytes buffer, [[maybe_unused]] Core::Stream::LocalSocket& socket) { InputMemoryStream stream { buffer }; u32 message_endpoint_magic = 0; @@ -685,7 +685,7 @@ public: message_generator.append(R"~~~( case (int)Messages::@endpoint.name@::MessageID::@message.pascal_name@: - message = Messages::@endpoint.name@::@message.pascal_name@::decode(stream, sockfd); + message = Messages::@endpoint.name@::@message.pascal_name@::decode(stream, socket); break; )~~~"); }; diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp index 7204b79d481..4e43bc93f2f 100644 --- a/Tests/LibCore/TestLibCoreStream.cpp +++ b/Tests/LibCore/TestLibCoreStream.cpp @@ -300,8 +300,8 @@ TEST_CASE(local_socket_read) auto local_server = Core::LocalServer::construct(); EXPECT(local_server->listen("/tmp/test-socket")); - local_server->on_accept = [&](NonnullRefPtr server_socket) { - EXPECT(server_socket->write(sent_data)); + local_server->on_accept = [&](NonnullOwnPtr server_socket) { + EXPECT(!server_socket->write(sent_data.bytes()).is_error()); event_loop.quit(0); event_loop.pump(); @@ -346,14 +346,17 @@ TEST_CASE(local_socket_write) auto local_server = Core::LocalServer::construct(); EXPECT(local_server->listen("/tmp/test-socket")); - local_server->on_accept = [&](NonnullRefPtr server_socket) { + local_server->on_accept = [&](NonnullOwnPtr server_socket) { // NOTE: For some reason LocalServer gives us a nonblocking socket..? - server_socket->set_blocking(true); + MUST(server_socket->set_blocking(true)); - auto receive_buffer = server_socket->read_all(); - EXPECT(!server_socket->error()); + auto pending_bytes = MUST(server_socket->pending_bytes()); + auto receive_buffer = ByteBuffer::create_uninitialized(pending_bytes).release_value(); + auto maybe_nread = server_socket->read(receive_buffer); + EXPECT(!maybe_nread.is_error()); + EXPECT(maybe_nread.value() == sent_data.length()); - StringView received_data { receive_buffer.bytes() }; + StringView received_data { receive_buffer.data(), maybe_nread.value() }; EXPECT_EQ(sent_data, received_data); event_loop.quit(0); diff --git a/Userland/Applets/Audio/main.cpp b/Userland/Applets/Audio/main.cpp index 40dfb5bbd54..8ce65dbe30a 100644 --- a/Userland/Applets/Audio/main.cpp +++ b/Userland/Applets/Audio/main.cpp @@ -41,14 +41,15 @@ public: { 0, TRY(Gfx::Bitmap::try_load_from_file("/res/icons/16x16/audio-volume-zero.png")) }, { 0, TRY(Gfx::Bitmap::try_load_from_file("/res/icons/16x16/audio-volume-muted.png")) } }; - NonnullRefPtr audio_widget = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AudioWidget(move(volume_level_bitmaps)))); + auto audio_client = TRY(Audio::ClientConnection::try_create()); + NonnullRefPtr audio_widget = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AudioWidget(move(audio_client), move(volume_level_bitmaps)))); TRY(audio_widget->try_initialize_graphical_elements()); return audio_widget; } private: - AudioWidget(Vector volume_level_bitmaps) - : m_audio_client(Audio::ClientConnection::construct()) + AudioWidget(NonnullRefPtr audio_client, Vector volume_level_bitmaps) + : m_audio_client(move(audio_client)) , m_volume_level_bitmaps(move(volume_level_bitmaps)) , m_show_percent(Config::read_bool("AudioApplet", "Applet", "ShowPercent", false)) { diff --git a/Userland/Applications/ImageViewer/ViewWidget.cpp b/Userland/Applications/ImageViewer/ViewWidget.cpp index 7cd0334e824..ebba2ba0535 100644 --- a/Userland/Applications/ImageViewer/ViewWidget.cpp +++ b/Userland/Applications/ImageViewer/ViewWidget.cpp @@ -169,7 +169,7 @@ void ViewWidget::load_from_file(const String& path) auto& mapped_file = *file_or_error.value(); // Spawn a new ImageDecoder service process and connect to it. - auto client = ImageDecoderClient::Client::construct(); + auto client = ImageDecoderClient::Client::try_create().release_value_but_fixme_should_propagate_errors(); auto decoded_image_or_error = client->decode_image(mapped_file.bytes()); if (!decoded_image_or_error.has_value()) { diff --git a/Userland/Applications/Piano/AudioPlayerLoop.cpp b/Userland/Applications/Piano/AudioPlayerLoop.cpp index 267117f08c6..5c7bdbb2da7 100644 --- a/Userland/Applications/Piano/AudioPlayerLoop.cpp +++ b/Userland/Applications/Piano/AudioPlayerLoop.cpp @@ -27,7 +27,7 @@ AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, bool& need_to_writ , m_need_to_write_wav(need_to_write_wav) , m_wav_writer(wav_writer) { - m_audio_client = Audio::ClientConnection::construct(); + m_audio_client = Audio::ClientConnection::try_create().release_value_but_fixme_should_propagate_errors(); m_audio_client->on_finish_playing_buffer = [this](int buffer_id) { (void)buffer_id; enqueue_audio(); diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp index 62aa9ac214d..246a9ef5389 100644 --- a/Userland/Applications/PixelPaint/Image.cpp +++ b/Userland/Applications/PixelPaint/Image.cpp @@ -56,7 +56,7 @@ void Image::paint_into(GUI::Painter& painter, Gfx::IntRect const& dest_rect) con ErrorOr> Image::try_decode_bitmap(ReadonlyBytes bitmap_data) { // Spawn a new ImageDecoder service process and connect to it. - auto client = ImageDecoderClient::Client::construct(); + auto client = TRY(ImageDecoderClient::Client::try_create()); // FIXME: Find a way to avoid the memory copying here. auto maybe_decoded_image = client->decode_image(bitmap_data); diff --git a/Userland/DevTools/HackStudio/LanguageClient.h b/Userland/DevTools/HackStudio/LanguageClient.h index 75eab5f0d8a..17a7420c2de 100644 --- a/Userland/DevTools/HackStudio/LanguageClient.h +++ b/Userland/DevTools/HackStudio/LanguageClient.h @@ -31,8 +31,8 @@ class ServerConnection friend class ServerConnectionWrapper; public: - ServerConnection(StringView socket, const String& project_path) - : IPC::ServerConnection(*this, socket) + ServerConnection(NonnullOwnPtr socket, const String& project_path) + : IPC::ServerConnection(*this, move(socket)) { m_project_path = project_path; async_greet(m_project_path); @@ -159,7 +159,7 @@ ServerConnectionWrapper& ServerConnectionWrapper::get_or_create(const String& pr if (wrapper) return *wrapper; - auto connection_wrapper_ptr = make(LanguageServerType::language_name(), [project_path]() { return LanguageServerType::construct(project_path); }); + auto connection_wrapper_ptr = make(LanguageServerType::language_name(), [project_path]() { return LanguageServerType::try_create(project_path).release_value_but_fixme_should_propagate_errors(); }); auto& connection_wrapper = *connection_wrapper_ptr; ServerConnectionInstances::set_instance_for_language(LanguageServerType::language_name(), move(connection_wrapper_ptr)); return connection_wrapper; diff --git a/Userland/DevTools/HackStudio/LanguageClients/ServerConnections.h b/Userland/DevTools/HackStudio/LanguageClients/ServerConnections.h index 9f88ae8acc6..aa705b19e30 100644 --- a/Userland/DevTools/HackStudio/LanguageClients/ServerConnections.h +++ b/Userland/DevTools/HackStudio/LanguageClients/ServerConnections.h @@ -12,19 +12,19 @@ #include #include -#define LANGUAGE_CLIENT(language_name_, socket_name) \ - namespace language_name_ { \ - class ServerConnection final : public HackStudio::ServerConnection { \ - C_OBJECT(ServerConnection) \ - public: \ - static const char* language_name() { return #language_name_; } \ - \ - private: \ - ServerConnection(const String& project_path) \ - : HackStudio::ServerConnection("/tmp/portal/language/" #socket_name, project_path) \ - { \ - } \ - }; \ +#define LANGUAGE_CLIENT(language_name_, socket_name) \ + namespace language_name_ { \ + class ServerConnection final : public HackStudio::ServerConnection { \ + IPC_CLIENT_CONNECTION(ServerConnection, "/tmp/portal/language" #socket_name) \ + public: \ + static const char* language_name() { return #language_name_; } \ + \ + private: \ + ServerConnection(NonnullOwnPtr socket, const String& project_path) \ + : HackStudio::ServerConnection(move(socket), project_path) \ + { \ + } \ + }; \ } namespace LanguageClients { diff --git a/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.cpp b/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.cpp index f7e21a2d369..8c5dfc05c6d 100644 --- a/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.cpp +++ b/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.cpp @@ -14,7 +14,7 @@ namespace LanguageServers { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) { s_connections.set(1, *this); diff --git a/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.h b/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.h index 951da34cacc..9394bc33e06 100644 --- a/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.h +++ b/Userland/DevTools/HackStudio/LanguageServers/ClientConnection.h @@ -20,7 +20,7 @@ namespace LanguageServers { class ClientConnection : public IPC::ClientConnection { public: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); ~ClientConnection() override; virtual void die() override; diff --git a/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.h b/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.h index c2fae04cca5..90cb30eb5b4 100644 --- a/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.h +++ b/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.h @@ -15,7 +15,7 @@ class ClientConnection final : public LanguageServers::ClientConnection { C_OBJECT(ClientConnection); private: - ClientConnection(NonnullRefPtr socket) + ClientConnection(NonnullOwnPtr socket) : LanguageServers::ClientConnection(move(socket)) { m_autocomplete_engine = make(m_filedb); diff --git a/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.h b/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.h index bedce254d92..8ca1eb66cc3 100644 --- a/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.h +++ b/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.h @@ -16,7 +16,7 @@ class ClientConnection final : public LanguageServers::ClientConnection { C_OBJECT(ClientConnection); private: - ClientConnection(NonnullRefPtr socket) + ClientConnection(NonnullOwnPtr socket) : LanguageServers::ClientConnection(move(socket)) { m_autocomplete_engine = make(m_filedb); diff --git a/Userland/DevTools/Inspector/InspectorServerClient.h b/Userland/DevTools/Inspector/InspectorServerClient.h index 926efac0066..f30c6a7a995 100644 --- a/Userland/DevTools/Inspector/InspectorServerClient.h +++ b/Userland/DevTools/Inspector/InspectorServerClient.h @@ -15,14 +15,14 @@ namespace Inspector { class InspectorServerClient final : public IPC::ServerConnection , public InspectorClientEndpoint { - C_OBJECT(InspectorServerClient); + IPC_CLIENT_CONNECTION(InspectorServerClient, "/tmp/portal/inspector") public: virtual ~InspectorServerClient() override = default; private: - InspectorServerClient() - : IPC::ServerConnection(*this, "/tmp/portal/inspector") + InspectorServerClient(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } }; diff --git a/Userland/DevTools/Inspector/RemoteProcess.cpp b/Userland/DevTools/Inspector/RemoteProcess.cpp index ae2b6b2a559..20227702f29 100644 --- a/Userland/DevTools/Inspector/RemoteProcess.cpp +++ b/Userland/DevTools/Inspector/RemoteProcess.cpp @@ -24,7 +24,7 @@ RemoteProcess::RemoteProcess(pid_t pid) , m_object_graph_model(RemoteObjectGraphModel::create(*this)) { s_the = this; - m_client = InspectorServerClient::construct(); + m_client = InspectorServerClient::try_create().release_value_but_fixme_should_propagate_errors(); } void RemoteProcess::handle_identify_response(const JsonObject& response) diff --git a/Userland/Libraries/LibAudio/ClientConnection.cpp b/Userland/Libraries/LibAudio/ClientConnection.cpp index 1261a0d0907..db3f5b0bcee 100644 --- a/Userland/Libraries/LibAudio/ClientConnection.cpp +++ b/Userland/Libraries/LibAudio/ClientConnection.cpp @@ -14,8 +14,8 @@ namespace Audio { // Real-time audio may be improved with a lower value. static timespec g_enqueue_wait_time { 0, 10'000'000 }; -ClientConnection::ClientConnection() - : IPC::ServerConnection(*this, "/tmp/portal/audio") +ClientConnection::ClientConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibAudio/ClientConnection.h b/Userland/Libraries/LibAudio/ClientConnection.h index 8ac59f64b18..8602d59b7fa 100644 --- a/Userland/Libraries/LibAudio/ClientConnection.h +++ b/Userland/Libraries/LibAudio/ClientConnection.h @@ -17,7 +17,7 @@ class Buffer; class ClientConnection final : public IPC::ServerConnection , public AudioClientEndpoint { - C_OBJECT(ClientConnection) + IPC_CLIENT_CONNECTION(ClientConnection, "/tmp/portal/audio") public: void enqueue(Buffer const&); bool try_enqueue(Buffer const&); @@ -29,7 +29,7 @@ public: Function on_client_volume_change; private: - ClientConnection(); + ClientConnection(NonnullOwnPtr); virtual void finished_playing_buffer(i32) override; virtual void main_mix_muted_state_changed(bool) override; diff --git a/Userland/Libraries/LibConfig/Client.cpp b/Userland/Libraries/LibConfig/Client.cpp index 3394d22b58c..7b821523dce 100644 --- a/Userland/Libraries/LibConfig/Client.cpp +++ b/Userland/Libraries/LibConfig/Client.cpp @@ -15,7 +15,7 @@ Client& Client::the() { if (!s_the || !s_the->is_open()) { VERIFY(Core::EventLoop::has_been_instantiated()); - s_the = Client::construct(); + s_the = Client::try_create().release_value_but_fixme_should_propagate_errors(); } return *s_the; } diff --git a/Userland/Libraries/LibConfig/Client.h b/Userland/Libraries/LibConfig/Client.h index 6f01a05aa28..4a7158e221a 100644 --- a/Userland/Libraries/LibConfig/Client.h +++ b/Userland/Libraries/LibConfig/Client.h @@ -18,7 +18,7 @@ namespace Config { class Client final : public IPC::ServerConnection , public ConfigClientEndpoint { - C_OBJECT(Client); + IPC_CLIENT_CONNECTION(Client, "/tmp/portal/config") public: void pledge_domains(Vector const&); @@ -39,8 +39,8 @@ public: static Client& the(); private: - explicit Client() - : IPC::ServerConnection(*this, "/tmp/portal/config") + explicit Client(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibCore/LocalServer.cpp b/Userland/Libraries/LibCore/LocalServer.cpp index aaba58baf9d..c76d4e28b90 100644 --- a/Userland/Libraries/LibCore/LocalServer.cpp +++ b/Userland/Libraries/LibCore/LocalServer.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -80,8 +81,13 @@ void LocalServer::setup_notifier() m_notifier = Notifier::construct(m_fd, Notifier::Event::Read, this); m_notifier->on_ready_to_read = [this] { if (on_accept) { - if (auto client_socket = accept()) - on_accept(client_socket.release_nonnull()); + auto maybe_client_socket = accept(); + if (maybe_client_socket.is_error()) { + dbgln("LocalServer::on_ready_to_read: Error accepting a connection: {} (FIXME: should propagate!)", maybe_client_socket.error()); + return; + } + + on_accept(maybe_client_socket.release_value()); } }; } @@ -134,7 +140,7 @@ bool LocalServer::listen(const String& address) return true; } -RefPtr LocalServer::accept() +ErrorOr> LocalServer::accept() { VERIFY(m_listening); sockaddr_un un; @@ -145,8 +151,7 @@ RefPtr LocalServer::accept() int accepted_fd = ::accept(m_fd, (sockaddr*)&un, &un_size); #endif if (accepted_fd < 0) { - perror("accept"); - return nullptr; + return Error::from_syscall("accept", -errno); } #ifdef AK_OS_MACOS @@ -155,7 +160,7 @@ RefPtr LocalServer::accept() (void)fcntl(accepted_fd, F_SETFD, FD_CLOEXEC); #endif - return LocalSocket::construct(accepted_fd); + return Stream::LocalSocket::adopt_fd(accepted_fd); } } diff --git a/Userland/Libraries/LibCore/LocalServer.h b/Userland/Libraries/LibCore/LocalServer.h index 18507502b60..209fb0547ba 100644 --- a/Userland/Libraries/LibCore/LocalServer.h +++ b/Userland/Libraries/LibCore/LocalServer.h @@ -8,6 +8,7 @@ #include #include +#include namespace Core { @@ -20,9 +21,9 @@ public: bool is_listening() const { return m_listening; } bool listen(const String& address); - RefPtr accept(); + ErrorOr> accept(); - Function)> on_accept; + Function)> on_accept; private: explicit LocalServer(Object* parent = nullptr); diff --git a/Userland/Libraries/LibDesktop/Launcher.cpp b/Userland/Libraries/LibDesktop/Launcher.cpp index 4af7c4c8de5..8c91d6adc87 100644 --- a/Userland/Libraries/LibDesktop/Launcher.cpp +++ b/Userland/Libraries/LibDesktop/Launcher.cpp @@ -36,17 +36,17 @@ auto Launcher::Details::from_details_str(const String& details_str) -> NonnullRe class LaunchServerConnection final : public IPC::ServerConnection , public LaunchClientEndpoint { - C_OBJECT(LaunchServerConnection) + IPC_CLIENT_CONNECTION(LaunchServerConnection, "/tmp/portal/launch") private: - LaunchServerConnection() - : IPC::ServerConnection(*this, "/tmp/portal/launch") + LaunchServerConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } }; static LaunchServerConnection& connection() { - static auto connection = LaunchServerConnection::construct(); + static auto connection = LaunchServerConnection::try_create().release_value_but_fixme_should_propagate_errors(); return connection; } diff --git a/Userland/Libraries/LibFileSystemAccessClient/Client.cpp b/Userland/Libraries/LibFileSystemAccessClient/Client.cpp index 730c1b609a7..a2a2b783ecf 100644 --- a/Userland/Libraries/LibFileSystemAccessClient/Client.cpp +++ b/Userland/Libraries/LibFileSystemAccessClient/Client.cpp @@ -20,7 +20,7 @@ static RefPtr s_the = nullptr; Client& Client::the() { if (!s_the || !s_the->is_open()) - s_the = Client::construct(); + s_the = Client::try_create().release_value_but_fixme_should_propagate_errors(); return *s_the; } diff --git a/Userland/Libraries/LibFileSystemAccessClient/Client.h b/Userland/Libraries/LibFileSystemAccessClient/Client.h index f816c0cab62..a12d0f03eb2 100644 --- a/Userland/Libraries/LibFileSystemAccessClient/Client.h +++ b/Userland/Libraries/LibFileSystemAccessClient/Client.h @@ -24,7 +24,7 @@ struct Result { class Client final : public IPC::ServerConnection , public FileSystemAccessClientEndpoint { - C_OBJECT(Client) + IPC_CLIENT_CONNECTION(Client, "/tmp/portal/filesystemaccess") public: Result request_file_read_only_approved(i32 parent_window_id, String const& path); @@ -38,8 +38,8 @@ protected: void die() override; private: - explicit Client() - : IPC::ServerConnection(*this, "/tmp/portal/filesystemaccess") + explicit Client(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibGUI/Clipboard.cpp b/Userland/Libraries/LibGUI/Clipboard.cpp index a3743195660..3bb941ac8db 100644 --- a/Userland/Libraries/LibGUI/Clipboard.cpp +++ b/Userland/Libraries/LibGUI/Clipboard.cpp @@ -16,11 +16,11 @@ namespace GUI { class ClipboardServerConnection final : public IPC::ServerConnection , public ClipboardClientEndpoint { - C_OBJECT(ClipboardServerConnection); + IPC_CLIENT_CONNECTION(ClipboardServerConnection, "/tmp/portal/clipboard") private: - ClipboardServerConnection() - : IPC::ServerConnection(*this, "/tmp/portal/clipboard") + ClipboardServerConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } @@ -30,7 +30,7 @@ private: } }; -static ClipboardServerConnection* s_connection; +static RefPtr s_connection; static ClipboardServerConnection& connection() { @@ -39,7 +39,7 @@ static ClipboardServerConnection& connection() void Clipboard::initialize(Badge) { - s_connection = &ClipboardServerConnection::construct().leak_ref(); + s_connection = ClipboardServerConnection::try_create().release_value_but_fixme_should_propagate_errors(); } Clipboard& Clipboard::the() diff --git a/Userland/Libraries/LibGUI/Notification.cpp b/Userland/Libraries/LibGUI/Notification.cpp index 5abd551dcbf..0f7e4207cb1 100644 --- a/Userland/Libraries/LibGUI/Notification.cpp +++ b/Userland/Libraries/LibGUI/Notification.cpp @@ -14,7 +14,7 @@ namespace GUI { class NotificationServerConnection final : public IPC::ServerConnection , public NotificationClientEndpoint { - C_OBJECT(NotificationServerConnection) + IPC_CLIENT_CONNECTION(NotificationServerConnection, "/tmp/portal/notify") friend class Notification; @@ -25,8 +25,8 @@ public: } private: - explicit NotificationServerConnection(Notification* notification) - : IPC::ServerConnection(*this, "/tmp/portal/notify") + explicit NotificationServerConnection(NonnullOwnPtr socket, Notification* notification) + : IPC::ServerConnection(*this, move(socket)) , m_notification(notification) { } @@ -45,7 +45,7 @@ void Notification::show() { VERIFY(!m_shown && !m_destroyed); auto icon = m_icon ? m_icon->to_shareable_bitmap() : Gfx::ShareableBitmap(); - m_connection = NotificationServerConnection::construct(this); + m_connection = NotificationServerConnection::try_create(this).release_value_but_fixme_should_propagate_errors(); m_connection->show_notification(m_text, m_title, icon); m_shown = true; } diff --git a/Userland/Libraries/LibGUI/WindowManagerServerConnection.cpp b/Userland/Libraries/LibGUI/WindowManagerServerConnection.cpp index bf6912fbcc9..8b68978cb1e 100644 --- a/Userland/Libraries/LibGUI/WindowManagerServerConnection.cpp +++ b/Userland/Libraries/LibGUI/WindowManagerServerConnection.cpp @@ -12,9 +12,9 @@ namespace GUI { WindowManagerServerConnection& WindowManagerServerConnection::the() { - static WindowManagerServerConnection* s_connection = nullptr; + static RefPtr s_connection = nullptr; if (!s_connection) - s_connection = new WindowManagerServerConnection; + s_connection = WindowManagerServerConnection::try_create().release_value_but_fixme_should_propagate_errors(); return *s_connection; } diff --git a/Userland/Libraries/LibGUI/WindowManagerServerConnection.h b/Userland/Libraries/LibGUI/WindowManagerServerConnection.h index 620e3cd0aa5..f4f2750bf8c 100644 --- a/Userland/Libraries/LibGUI/WindowManagerServerConnection.h +++ b/Userland/Libraries/LibGUI/WindowManagerServerConnection.h @@ -16,13 +16,14 @@ namespace GUI { class WindowManagerServerConnection final : public IPC::ServerConnection , public WindowManagerClientEndpoint { - C_OBJECT(WindowManagerServerConnection) + IPC_CLIENT_CONNECTION(WindowManagerServerConnection, "/tmp/portal/wm") + public: static WindowManagerServerConnection& the(); private: - WindowManagerServerConnection() - : IPC::ServerConnection(*this, "/tmp/portal/wm") + WindowManagerServerConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibGUI/WindowServerConnection.cpp b/Userland/Libraries/LibGUI/WindowServerConnection.cpp index 32e93e18d7d..60490a41294 100644 --- a/Userland/Libraries/LibGUI/WindowServerConnection.cpp +++ b/Userland/Libraries/LibGUI/WindowServerConnection.cpp @@ -28,9 +28,9 @@ namespace GUI { WindowServerConnection& WindowServerConnection::the() { - static WindowServerConnection* s_connection = nullptr; + static RefPtr s_connection = nullptr; if (!s_connection) - s_connection = new WindowServerConnection; + s_connection = WindowServerConnection::try_create().release_value_but_fixme_should_propagate_errors(); return *s_connection; } @@ -40,8 +40,8 @@ static void set_system_theme_from_anonymous_buffer(Core::AnonymousBuffer buffer) Application::the()->set_system_palette(buffer); } -WindowServerConnection::WindowServerConnection() - : IPC::ServerConnection(*this, "/tmp/portal/window") +WindowServerConnection::WindowServerConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { // NOTE: WindowServer automatically sends a "fast_greet" message to us when we connect. // All we have to do is wait for it to arrive. This avoids a round-trip during application startup. diff --git a/Userland/Libraries/LibGUI/WindowServerConnection.h b/Userland/Libraries/LibGUI/WindowServerConnection.h index 68d768e9158..24adb18407c 100644 --- a/Userland/Libraries/LibGUI/WindowServerConnection.h +++ b/Userland/Libraries/LibGUI/WindowServerConnection.h @@ -16,13 +16,13 @@ namespace GUI { class WindowServerConnection final : public IPC::ServerConnection , public WindowClientEndpoint { - C_OBJECT(WindowServerConnection) + IPC_CLIENT_CONNECTION(WindowServerConnection, "/tmp/portal/window") public: static WindowServerConnection& the(); i32 expose_client_id() { return m_client_id; } private: - WindowServerConnection(); + WindowServerConnection(NonnullOwnPtr); virtual void fast_greet(Vector const&, u32, u32, u32, Core::AnonymousBuffer const&, String const&, String const&, i32) override; virtual void paint(i32, Gfx::IntSize const&, Vector const&) override; diff --git a/Userland/Libraries/LibIPC/CMakeLists.txt b/Userland/Libraries/LibIPC/CMakeLists.txt index 9b41b56af79..1fbb221b3a1 100644 --- a/Userland/Libraries/LibIPC/CMakeLists.txt +++ b/Userland/Libraries/LibIPC/CMakeLists.txt @@ -4,6 +4,7 @@ set(SOURCES Encoder.cpp Message.cpp Stub.cpp + SystemServerTakeover.cpp ) serenity_lib(LibIPC ipc) diff --git a/Userland/Libraries/LibIPC/ClientConnection.h b/Userland/Libraries/LibIPC/ClientConnection.h index 9d918dca922..2fef3b83ef1 100644 --- a/Userland/Libraries/LibIPC/ClientConnection.h +++ b/Userland/Libraries/LibIPC/ClientConnection.h @@ -24,12 +24,12 @@ public: using ServerStub = typename ServerEndpoint::Stub; using IPCProxy = typename ClientEndpoint::template Proxy; - ClientConnection(ServerStub& stub, NonnullRefPtr socket, int client_id) + ClientConnection(ServerStub& stub, NonnullOwnPtr socket, int client_id) : IPC::Connection(stub, move(socket)) , ClientEndpoint::template Proxy(*this, {}) , m_client_id(client_id) { - VERIFY(this->socket().is_connected()); + VERIFY(this->socket().is_open()); this->socket().on_ready_to_read = [this] { // FIXME: Do something about errors. (void)this->drain_messages_from_peer(); diff --git a/Userland/Libraries/LibIPC/Connection.cpp b/Userland/Libraries/LibIPC/Connection.cpp index d82aab53a57..07b39345ff4 100644 --- a/Userland/Libraries/LibIPC/Connection.cpp +++ b/Userland/Libraries/LibIPC/Connection.cpp @@ -11,10 +11,9 @@ namespace IPC { -ConnectionBase::ConnectionBase(IPC::Stub& local_stub, NonnullRefPtr socket, u32 local_endpoint_magic) +ConnectionBase::ConnectionBase(IPC::Stub& local_stub, NonnullOwnPtr socket, u32 local_endpoint_magic) : m_local_stub(local_stub) , m_socket(move(socket)) - , m_notifier(Core::Notifier::construct(m_socket->fd(), Core::Notifier::Read, this)) , m_local_endpoint_magic(local_endpoint_magic) { m_responsiveness_timer = Core::Timer::create_single_shot(3000, [this] { may_have_become_unresponsive(); }); @@ -42,7 +41,7 @@ ErrorOr ConnectionBase::post_message(MessageBuffer buffer) #ifdef __serenity__ for (auto& fd : buffer.fds) { - if (auto result = Core::System::sendfd(m_socket->fd(), fd.value()); result.is_error()) { + if (auto result = m_socket->send_fd(fd.value()); result.is_error()) { dbgln("{}", result.error()); shutdown(); return result; @@ -53,23 +52,29 @@ ErrorOr ConnectionBase::post_message(MessageBuffer buffer) warnln("fd passing is not supported on this platform, sorry :("); #endif - size_t total_nwritten = 0; - while (total_nwritten < buffer.data.size()) { - auto nwritten = write(m_socket->fd(), buffer.data.data() + total_nwritten, buffer.data.size() - total_nwritten); - if (nwritten < 0) { - switch (errno) { - case EPIPE: - shutdown(); - return Error::from_string_literal("IPC::Connection::post_message: Disconnected from peer"sv); - case EAGAIN: - shutdown(); - return Error::from_string_literal("IPC::Connection::post_message: Peer buffer overflowed"sv); - default: - shutdown(); - return Error::from_syscall("IPC::Connection::post_message write"sv, -errno); + ReadonlyBytes bytes_to_write { buffer.data.span() }; + while (!bytes_to_write.is_empty()) { + auto maybe_nwritten = m_socket->write(bytes_to_write); + if (maybe_nwritten.is_error()) { + auto error = maybe_nwritten.release_error(); + if (error.is_errno()) { + switch (error.code()) { + case EPIPE: + shutdown(); + return Error::from_string_literal("IPC::Connection::post_message: Disconnected from peer"sv); + case EAGAIN: + shutdown(); + return Error::from_string_literal("IPC::Connection::post_message: Peer buffer overflowed"sv); + default: + shutdown(); + return Error::from_syscall("IPC::Connection::post_message write"sv, -error.code()); + } + } else { + return error; } } - total_nwritten += nwritten; + + bytes_to_write = bytes_to_write.slice(maybe_nwritten.value()); } m_responsiveness_timer->start(); @@ -78,7 +83,6 @@ ErrorOr ConnectionBase::post_message(MessageBuffer buffer) void ConnectionBase::shutdown() { - m_notifier->close(); m_socket->close(); die(); } @@ -99,21 +103,14 @@ void ConnectionBase::handle_messages() void ConnectionBase::wait_for_socket_to_become_readable() { - fd_set read_fds; - FD_ZERO(&read_fds); - FD_SET(m_socket->fd(), &read_fds); - for (;;) { - if (auto rc = select(m_socket->fd() + 1, &read_fds, nullptr, nullptr, nullptr); rc < 0) { - if (errno == EINTR) - continue; - perror("wait_for_specific_endpoint_message: select"); - VERIFY_NOT_REACHED(); - } else { - VERIFY(rc > 0); - VERIFY(FD_ISSET(m_socket->fd(), &read_fds)); - break; - } + auto maybe_did_become_readable = m_socket->can_read_without_blocking(-1); + if (maybe_did_become_readable.is_error()) { + dbgln("ConnectionBase::wait_for_socket_to_become_readable: {}", maybe_did_become_readable.error()); + warnln("ConnectionBase::wait_for_socket_to_become_readable: {}", maybe_did_become_readable.error()); + VERIFY_NOT_REACHED(); } + + VERIFY(maybe_did_become_readable.value()); } ErrorOr> ConnectionBase::read_as_much_as_possible_from_socket_without_blocking() @@ -125,15 +122,21 @@ ErrorOr> ConnectionBase::read_as_much_as_possible_from_socket_without m_unprocessed_bytes.clear(); } + u8 buffer[4096]; while (m_socket->is_open()) { - u8 buffer[4096]; - ssize_t nread = recv(m_socket->fd(), buffer, sizeof(buffer), MSG_DONTWAIT); - if (nread < 0) { - if (errno == EAGAIN) + auto maybe_nread = m_socket->read_without_waiting({ buffer, 4096 }); + if (maybe_nread.is_error()) { + auto error = maybe_nread.release_error(); + if (error.is_syscall() && error.code() == EAGAIN) { break; - perror("recv"); - exit(1); + } + + dbgln("ConnectionBase::read_as_much_as_possible_from_socket_without_blocking: {}", error); + warnln("ConnectionBase::read_as_much_as_possible_from_socket_without_blocking: {}", error); + VERIFY_NOT_REACHED(); } + + auto nread = maybe_nread.release_value(); if (nread == 0) { if (bytes.is_empty()) { deferred_invoke([this] { shutdown(); }); @@ -141,6 +144,7 @@ ErrorOr> ConnectionBase::read_as_much_as_possible_from_socket_without } break; } + bytes.append(buffer, nread); } diff --git a/Userland/Libraries/LibIPC/Connection.h b/Userland/Libraries/LibIPC/Connection.h index d8a46e89c0d..f6831f86199 100644 --- a/Userland/Libraries/LibIPC/Connection.h +++ b/Userland/Libraries/LibIPC/Connection.h @@ -11,8 +11,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -39,9 +39,9 @@ public: virtual void die() { } protected: - explicit ConnectionBase(IPC::Stub&, NonnullRefPtr, u32 local_endpoint_magic); + explicit ConnectionBase(IPC::Stub&, NonnullOwnPtr, u32 local_endpoint_magic); - Core::LocalSocket& socket() { return *m_socket; } + Core::Stream::LocalSocket& socket() { return *m_socket; } virtual void may_have_become_unresponsive() { } virtual void did_become_responsive() { } @@ -57,10 +57,9 @@ protected: IPC::Stub& m_local_stub; - NonnullRefPtr m_socket; + NonnullOwnPtr m_socket; RefPtr m_responsiveness_timer; - RefPtr m_notifier; NonnullOwnPtrVector m_unprocessed_messages; ByteBuffer m_unprocessed_bytes; @@ -70,10 +69,10 @@ protected: template class Connection : public ConnectionBase { public: - Connection(IPC::Stub& local_stub, NonnullRefPtr socket) + Connection(IPC::Stub& local_stub, NonnullOwnPtr socket) : ConnectionBase(local_stub, move(socket), LocalEndpoint::static_magic()) { - m_notifier->on_ready_to_read = [this] { + m_socket->on_ready_to_read = [this] { NonnullRefPtr protect = *this; // FIXME: Do something about errors. (void)drain_messages_from_peer(); @@ -122,9 +121,9 @@ protected: break; index += sizeof(message_size); auto remaining_bytes = ReadonlyBytes { bytes.data() + index, message_size }; - if (auto message = LocalEndpoint::decode_message(remaining_bytes, m_socket->fd())) { + if (auto message = LocalEndpoint::decode_message(remaining_bytes, *m_socket)) { m_unprocessed_messages.append(message.release_nonnull()); - } else if (auto message = PeerEndpoint::decode_message(remaining_bytes, m_socket->fd())) { + } else if (auto message = PeerEndpoint::decode_message(remaining_bytes, *m_socket)) { m_unprocessed_messages.append(message.release_nonnull()); } else { dbgln("Failed to parse a message"); diff --git a/Userland/Libraries/LibIPC/Decoder.cpp b/Userland/Libraries/LibIPC/Decoder.cpp index 19fc53ef5a5..75363fded0c 100644 --- a/Userland/Libraries/LibIPC/Decoder.cpp +++ b/Userland/Libraries/LibIPC/Decoder.cpp @@ -162,14 +162,9 @@ ErrorOr Decoder::decode(Dictionary& dictionary) ErrorOr Decoder::decode([[maybe_unused]] File& file) { -#ifdef __serenity__ - int fd = TRY(Core::System::recvfd(m_sockfd, O_CLOEXEC)); + int fd = TRY(m_socket.receive_fd(O_CLOEXEC)); file = File(fd, File::ConstructWithReceivedFileDescriptor); return {}; -#else - [[maybe_unused]] auto fd = m_sockfd; - return Error::from_string_literal("File descriptor passing not supported on this platform"); -#endif } ErrorOr decode(Decoder& decoder, Core::AnonymousBuffer& buffer) diff --git a/Userland/Libraries/LibIPC/Decoder.h b/Userland/Libraries/LibIPC/Decoder.h index c36e4a2a52b..8f41894b750 100644 --- a/Userland/Libraries/LibIPC/Decoder.h +++ b/Userland/Libraries/LibIPC/Decoder.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -25,9 +26,9 @@ inline ErrorOr decode(Decoder&, T&) class Decoder { public: - Decoder(InputMemoryStream& stream, int sockfd) + Decoder(InputMemoryStream& stream, Core::Stream::LocalSocket& socket) : m_stream(stream) - , m_sockfd(sockfd) + , m_socket(socket) { } @@ -115,7 +116,7 @@ public: private: InputMemoryStream& m_stream; - int m_sockfd { -1 }; + Core::Stream::LocalSocket& m_socket; }; } diff --git a/Userland/Libraries/LibIPC/ServerConnection.h b/Userland/Libraries/LibIPC/ServerConnection.h index 3d6b78dba82..394f8b6630c 100644 --- a/Userland/Libraries/LibIPC/ServerConnection.h +++ b/Userland/Libraries/LibIPC/ServerConnection.h @@ -6,10 +6,24 @@ #pragma once +#include #include namespace IPC { +#define IPC_CLIENT_CONNECTION(klass, socket_path) \ + C_OBJECT_ABSTRACT(klass) \ +public: \ + template \ + static ErrorOr> try_create(Args&&... args) \ + { \ + auto socket = TRY(Core::Stream::LocalSocket::connect(socket_path)); \ + /* We want to rate-limit our clients */ \ + TRY(socket->set_blocking(true)); \ + \ + return adopt_nonnull_ref_or_enomem(new (nothrow) Klass(move(socket), forward(args)...)); \ + } + template class ServerConnection : public IPC::Connection , public ClientEndpoint::Stub @@ -18,19 +32,10 @@ public: using ClientStub = typename ClientEndpoint::Stub; using IPCProxy = typename ServerEndpoint::template Proxy; - ServerConnection(ClientStub& local_endpoint, StringView address) - : Connection(local_endpoint, Core::LocalSocket::construct()) + ServerConnection(ClientStub& local_endpoint, NonnullOwnPtr socket) + : Connection(local_endpoint, move(socket)) , ServerEndpoint::template Proxy(*this, {}) { - // We want to rate-limit our clients - this->socket().set_blocking(true); - - if (!this->socket().connect(Core::SocketAddress::local(address))) { - perror("connect"); - VERIFY_NOT_REACHED(); - } - - VERIFY(this->socket().is_connected()); } virtual void die() override diff --git a/Userland/Libraries/LibIPC/SingleServer.h b/Userland/Libraries/LibIPC/SingleServer.h index 36477ae40a7..de80b4db156 100644 --- a/Userland/Libraries/LibIPC/SingleServer.h +++ b/Userland/Libraries/LibIPC/SingleServer.h @@ -6,14 +6,16 @@ #pragma once +#include #include +#include namespace IPC { template ErrorOr> take_over_accepted_client_from_system_server() { - auto socket = TRY(Core::LocalSocket::take_over_accepted_socket_from_system_server()); + auto socket = TRY(take_over_accepted_socket_from_system_server()); return IPC::new_client_connection(move(socket)); } diff --git a/Userland/Libraries/LibIPC/SystemServerTakeover.cpp b/Userland/Libraries/LibIPC/SystemServerTakeover.cpp new file mode 100644 index 00000000000..0948c42ca9e --- /dev/null +++ b/Userland/Libraries/LibIPC/SystemServerTakeover.cpp @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2022, sin-ack + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "SystemServerTakeover.h" +#include + +HashMap s_overtaken_sockets {}; +bool s_overtaken_sockets_parsed { false }; + +void parse_sockets_from_system_server() +{ + VERIFY(!s_overtaken_sockets_parsed); + + constexpr auto socket_takeover = "SOCKET_TAKEOVER"; + const char* sockets = getenv(socket_takeover); + if (!sockets) { + s_overtaken_sockets_parsed = true; + return; + } + + for (auto& socket : StringView(sockets).split_view(' ')) { + auto params = socket.split_view(':'); + s_overtaken_sockets.set(params[0].to_string(), strtol(params[1].to_string().characters(), nullptr, 10)); + } + + s_overtaken_sockets_parsed = true; + // We wouldn't want our children to think we're passing + // them a socket either, so unset the env variable. + unsetenv(socket_takeover); +} + +ErrorOr> take_over_accepted_socket_from_system_server(String const& socket_path) +{ + if (!s_overtaken_sockets_parsed) + parse_sockets_from_system_server(); + + int fd; + if (socket_path.is_null()) { + // We want the first (and only) socket. + VERIFY(s_overtaken_sockets.size() == 1); + fd = s_overtaken_sockets.begin()->value; + } else { + auto it = s_overtaken_sockets.find(socket_path); + if (it == s_overtaken_sockets.end()) + return Error::from_string_literal("Non-existent socket requested"sv); + fd = it->value; + } + + // Sanity check: it has to be a socket. + auto stat = TRY(Core::System::fstat(fd)); + + if (!S_ISSOCK(stat.st_mode)) + return Error::from_string_literal("The fd we got from SystemServer is not a socket"sv); + + auto socket = TRY(Core::Stream::LocalSocket::adopt_fd(fd)); + // It had to be !CLOEXEC for obvious reasons, but we + // don't need it to be !CLOEXEC anymore, so set the + // CLOEXEC flag now. + TRY(socket->set_close_on_exec(true)); + + return socket; +} diff --git a/Userland/Libraries/LibIPC/SystemServerTakeover.h b/Userland/Libraries/LibIPC/SystemServerTakeover.h new file mode 100644 index 00000000000..5dbda758b14 --- /dev/null +++ b/Userland/Libraries/LibIPC/SystemServerTakeover.h @@ -0,0 +1,12 @@ +/* + * Copyright (c) 2022, sin-ack + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +void parse_sockets_from_system_server(); +ErrorOr> take_over_accepted_socket_from_system_server(String const& socket_path = {}); diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp index 4a46bef119f..417e6869e3d 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.cpp +++ b/Userland/Libraries/LibImageDecoderClient/Client.cpp @@ -9,8 +9,8 @@ namespace ImageDecoderClient { -Client::Client() - : IPC::ServerConnection(*this, "/tmp/portal/image") +Client::Client(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibImageDecoderClient/Client.h b/Userland/Libraries/LibImageDecoderClient/Client.h index 6dddb8eaf53..7a4ac22148c 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.h +++ b/Userland/Libraries/LibImageDecoderClient/Client.h @@ -27,7 +27,7 @@ struct DecodedImage { class Client final : public IPC::ServerConnection , public ImageDecoderClientEndpoint { - C_OBJECT(Client); + IPC_CLIENT_CONNECTION(Client, "/tmp/portal/image"); public: Optional decode_image(ReadonlyBytes); @@ -35,7 +35,7 @@ public: Function on_death; private: - Client(); + Client(NonnullOwnPtr); virtual void die() override; }; diff --git a/Userland/Libraries/LibProtocol/RequestClient.cpp b/Userland/Libraries/LibProtocol/RequestClient.cpp index 14494683345..ac394def1f2 100644 --- a/Userland/Libraries/LibProtocol/RequestClient.cpp +++ b/Userland/Libraries/LibProtocol/RequestClient.cpp @@ -10,8 +10,8 @@ namespace Protocol { -RequestClient::RequestClient() - : IPC::ServerConnection(*this, "/tmp/portal/request") +RequestClient::RequestClient(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibProtocol/RequestClient.h b/Userland/Libraries/LibProtocol/RequestClient.h index 82248e9e3a7..74b4fc8b1e4 100644 --- a/Userland/Libraries/LibProtocol/RequestClient.h +++ b/Userland/Libraries/LibProtocol/RequestClient.h @@ -18,7 +18,7 @@ class Request; class RequestClient final : public IPC::ServerConnection , public RequestClientEndpoint { - C_OBJECT(RequestClient); + IPC_CLIENT_CONNECTION(RequestClient, "/tmp/portal/request") public: template> @@ -30,7 +30,7 @@ public: bool set_certificate(Badge, Request&, String, String); private: - RequestClient(); + RequestClient(NonnullOwnPtr); virtual void request_progress(i32, Optional const&, u32) override; virtual void request_finished(i32, bool, u32) override; diff --git a/Userland/Libraries/LibProtocol/WebSocketClient.cpp b/Userland/Libraries/LibProtocol/WebSocketClient.cpp index da1b4318a45..9a57235d608 100644 --- a/Userland/Libraries/LibProtocol/WebSocketClient.cpp +++ b/Userland/Libraries/LibProtocol/WebSocketClient.cpp @@ -9,8 +9,8 @@ namespace Protocol { -WebSocketClient::WebSocketClient() - : IPC::ServerConnection(*this, "/tmp/portal/websocket") +WebSocketClient::WebSocketClient(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibProtocol/WebSocketClient.h b/Userland/Libraries/LibProtocol/WebSocketClient.h index c2f8a70ce6a..57a0e9c276b 100644 --- a/Userland/Libraries/LibProtocol/WebSocketClient.h +++ b/Userland/Libraries/LibProtocol/WebSocketClient.h @@ -18,7 +18,7 @@ class WebSocket; class WebSocketClient final : public IPC::ServerConnection , public WebSocketClientEndpoint { - C_OBJECT(WebSocketClient); + IPC_CLIENT_CONNECTION(WebSocketClient, "/tmp/portal/websocket") public: RefPtr connect(const URL&, const String& origin = {}, const Vector& protocols = {}, const Vector& extensions = {}, const HashMap& request_headers = {}); @@ -29,7 +29,7 @@ public: bool set_certificate(Badge, WebSocket&, String, String); private: - WebSocketClient(); + WebSocketClient(NonnullOwnPtr); virtual void connected(i32) override; virtual void received(i32, bool, ByteBuffer const&) override; diff --git a/Userland/Libraries/LibSQL/SQLClient.h b/Userland/Libraries/LibSQL/SQLClient.h index d1947d6cde7..b6884e03ca4 100644 --- a/Userland/Libraries/LibSQL/SQLClient.h +++ b/Userland/Libraries/LibSQL/SQLClient.h @@ -15,7 +15,7 @@ namespace SQL { class SQLClient : public IPC::ServerConnection , public SQLClientEndpoint { - C_OBJECT(SQLClient); + IPC_CLIENT_CONNECTION(SQLClient, "/tmp/portal/sql") virtual ~SQLClient(); Function on_connected; @@ -27,8 +27,8 @@ class SQLClient Function on_results_exhausted; private: - SQLClient() - : IPC::ServerConnection(*this, "/tmp/portal/sql") + SQLClient(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } diff --git a/Userland/Libraries/LibWeb/HTML/WebSocket.cpp b/Userland/Libraries/LibWeb/HTML/WebSocket.cpp index 5094174577e..5555b534715 100644 --- a/Userland/Libraries/LibWeb/HTML/WebSocket.cpp +++ b/Userland/Libraries/LibWeb/HTML/WebSocket.cpp @@ -29,14 +29,20 @@ namespace Web::HTML { WebSocketClientManager& WebSocketClientManager::the() { - static WebSocketClientManager* s_the; + static RefPtr s_the; if (!s_the) - s_the = &WebSocketClientManager::construct().leak_ref(); + s_the = WebSocketClientManager::try_create().release_value_but_fixme_should_propagate_errors(); return *s_the; } -WebSocketClientManager::WebSocketClientManager() - : m_websocket_client(Protocol::WebSocketClient::construct()) +ErrorOr> WebSocketClientManager::try_create() +{ + auto websocket_client = TRY(Protocol::WebSocketClient::try_create()); + return adopt_nonnull_ref_or_enomem(new (nothrow) WebSocketClientManager(move(websocket_client))); +} + +WebSocketClientManager::WebSocketClientManager(NonnullRefPtr websocket_client) + : m_websocket_client(move(websocket_client)) { } diff --git a/Userland/Libraries/LibWeb/HTML/WebSocket.h b/Userland/Libraries/LibWeb/HTML/WebSocket.h index 4748b5dc477..ba4990e3d50 100644 --- a/Userland/Libraries/LibWeb/HTML/WebSocket.h +++ b/Userland/Libraries/LibWeb/HTML/WebSocket.h @@ -31,14 +31,16 @@ class WebSocket; namespace Web::HTML { class WebSocketClientManager : public Core::Object { - C_OBJECT(WebSocketClientManager) + C_OBJECT_ABSTRACT(WebSocketClientManager) public: static WebSocketClientManager& the(); RefPtr connect(const AK::URL&); private: - WebSocketClientManager(); + static ErrorOr> try_create(); + WebSocketClientManager(NonnullRefPtr); + RefPtr m_websocket_client; }; diff --git a/Userland/Libraries/LibWeb/ImageDecoding.cpp b/Userland/Libraries/LibWeb/ImageDecoding.cpp index 706c04c97ad..44967a88600 100644 --- a/Userland/Libraries/LibWeb/ImageDecoding.cpp +++ b/Userland/Libraries/LibWeb/ImageDecoding.cpp @@ -12,7 +12,7 @@ ImageDecoderClient::Client& image_decoder_client() { static RefPtr image_decoder_client; if (!image_decoder_client) { - image_decoder_client = ImageDecoderClient::Client::construct(); + image_decoder_client = ImageDecoderClient::Client::try_create().release_value_but_fixme_should_propagate_errors(); image_decoder_client->on_death = [&] { image_decoder_client = nullptr; }; diff --git a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp index e87e674d98c..cb41f1ea73b 100644 --- a/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp +++ b/Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp @@ -21,14 +21,21 @@ namespace Web { ResourceLoader& ResourceLoader::the() { - static ResourceLoader* s_the; + static RefPtr s_the; if (!s_the) - s_the = &ResourceLoader::construct().leak_ref(); + s_the = ResourceLoader::try_create().release_value_but_fixme_should_propagate_errors(); return *s_the; } -ResourceLoader::ResourceLoader() - : m_protocol_client(Protocol::RequestClient::construct()) +ErrorOr> ResourceLoader::try_create() +{ + + auto protocol_client = TRY(Protocol::RequestClient::try_create()); + return adopt_nonnull_ref_or_enomem(new (nothrow) ResourceLoader(move(protocol_client))); +} + +ResourceLoader::ResourceLoader(NonnullRefPtr protocol_client) + : m_protocol_client(move(protocol_client)) , m_user_agent(default_user_agent) { } diff --git a/Userland/Libraries/LibWeb/Loader/ResourceLoader.h b/Userland/Libraries/LibWeb/Loader/ResourceLoader.h index e311baddea8..6475dbe3da6 100644 --- a/Userland/Libraries/LibWeb/Loader/ResourceLoader.h +++ b/Userland/Libraries/LibWeb/Loader/ResourceLoader.h @@ -27,7 +27,7 @@ namespace Web { constexpr auto default_user_agent = "Mozilla/4.0 (SerenityOS; " CPU_STRING ") LibWeb+LibJS (Not KHTML, nor Gecko) LibWeb"; class ResourceLoader : public Core::Object { - C_OBJECT(ResourceLoader) + C_OBJECT_ABSTRACT(ResourceLoader) public: static ResourceLoader& the(); @@ -52,7 +52,9 @@ public: void clear_cache(); private: - ResourceLoader(); + ResourceLoader(NonnullRefPtr protocol_client); + static ErrorOr> try_create(); + static bool is_port_blocked(int port); int m_pending_loads { 0 }; diff --git a/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp b/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp index a3aa229c223..b9390ca6182 100644 --- a/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp +++ b/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp @@ -62,7 +62,7 @@ void OutOfProcessWebView::create_client() { m_client_state = {}; - m_client_state.client = WebContentClient::construct(*this); + m_client_state.client = WebContentClient::try_create(*this).release_value_but_fixme_should_propagate_errors(); m_client_state.client->on_web_content_process_crash = [this] { deferred_invoke([this] { handle_web_content_process_crash(); diff --git a/Userland/Libraries/LibWeb/WebContentClient.cpp b/Userland/Libraries/LibWeb/WebContentClient.cpp index e952bfb2fbb..770fed6e65f 100644 --- a/Userland/Libraries/LibWeb/WebContentClient.cpp +++ b/Userland/Libraries/LibWeb/WebContentClient.cpp @@ -11,8 +11,8 @@ namespace Web { -WebContentClient::WebContentClient(OutOfProcessWebView& view) - : IPC::ServerConnection(*this, "/tmp/portal/webcontent") +WebContentClient::WebContentClient(NonnullOwnPtr socket, OutOfProcessWebView& view) + : IPC::ServerConnection(*this, move(socket)) , m_view(view) { } diff --git a/Userland/Libraries/LibWeb/WebContentClient.h b/Userland/Libraries/LibWeb/WebContentClient.h index 27dba94eb30..14ded8ebf1f 100644 --- a/Userland/Libraries/LibWeb/WebContentClient.h +++ b/Userland/Libraries/LibWeb/WebContentClient.h @@ -19,13 +19,13 @@ class OutOfProcessWebView; class WebContentClient final : public IPC::ServerConnection , public WebContentClientEndpoint { - C_OBJECT(WebContentClient); + IPC_CLIENT_CONNECTION(WebContentClient, "/tmp/portal/webcontent"); public: Function on_web_content_process_crash; private: - WebContentClient(OutOfProcessWebView&); + WebContentClient(NonnullOwnPtr, OutOfProcessWebView&); virtual void die() override; diff --git a/Userland/Services/AudioServer/ClientConnection.cpp b/Userland/Services/AudioServer/ClientConnection.cpp index 825413726ed..758ecde1b30 100644 --- a/Userland/Services/AudioServer/ClientConnection.cpp +++ b/Userland/Services/AudioServer/ClientConnection.cpp @@ -22,7 +22,7 @@ void ClientConnection::for_each(Function callback) callback(connection); } -ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id, Mixer& mixer) +ClientConnection::ClientConnection(NonnullOwnPtr client_socket, int client_id, Mixer& mixer) : IPC::ClientConnection(*this, move(client_socket), client_id) , m_mixer(mixer) { diff --git a/Userland/Services/AudioServer/ClientConnection.h b/Userland/Services/AudioServer/ClientConnection.h index 878c4bebd30..58109dbab3d 100644 --- a/Userland/Services/AudioServer/ClientConnection.h +++ b/Userland/Services/AudioServer/ClientConnection.h @@ -35,7 +35,7 @@ public: static void for_each(Function); private: - explicit ClientConnection(NonnullRefPtr, int client_id, Mixer& mixer); + explicit ClientConnection(NonnullOwnPtr, int client_id, Mixer& mixer); virtual Messages::AudioServer::GetMainMixVolumeResponse get_main_mix_volume() override; virtual void set_main_mix_volume(double) override; diff --git a/Userland/Services/AudioServer/main.cpp b/Userland/Services/AudioServer/main.cpp index 621f3940245..dd8da231477 100644 --- a/Userland/Services/AudioServer/main.cpp +++ b/Userland/Services/AudioServer/main.cpp @@ -25,7 +25,7 @@ ErrorOr serenity_main(Main::Arguments) auto server = TRY(Core::LocalServer::try_create()); TRY(server->take_over_from_system_server()); - server->on_accept = [&](NonnullRefPtr client_socket) { + server->on_accept = [&](NonnullOwnPtr client_socket) { static int s_next_client_id = 0; int client_id = ++s_next_client_id; (void)IPC::new_client_connection(move(client_socket), client_id, *mixer); diff --git a/Userland/Services/Clipboard/ClientConnection.cpp b/Userland/Services/Clipboard/ClientConnection.cpp index aef4f1cf6f3..830af96e973 100644 --- a/Userland/Services/Clipboard/ClientConnection.cpp +++ b/Userland/Services/Clipboard/ClientConnection.cpp @@ -19,7 +19,7 @@ void ClientConnection::for_each_client(Function callbac } } -ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr socket, int client_id) : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/Clipboard/ClientConnection.h b/Userland/Services/Clipboard/ClientConnection.h index ee53f8d9a98..2ec5037f35f 100644 --- a/Userland/Services/Clipboard/ClientConnection.h +++ b/Userland/Services/Clipboard/ClientConnection.h @@ -27,7 +27,7 @@ public: void notify_about_clipboard_change(); private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual Messages::ClipboardServer::GetClipboardDataResponse get_clipboard_data() override; virtual void set_clipboard_data(Core::AnonymousBuffer const&, String const&, IPC::Dictionary const&) override; diff --git a/Userland/Services/ConfigServer/ClientConnection.cpp b/Userland/Services/ConfigServer/ClientConnection.cpp index d52e2bf2374..75dab718ae1 100644 --- a/Userland/Services/ConfigServer/ClientConnection.cpp +++ b/Userland/Services/ConfigServer/ClientConnection.cpp @@ -74,7 +74,7 @@ static Core::ConfigFile& ensure_domain_config(String const& domain) return *config; } -ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) , m_sync_timer(Core::Timer::create_single_shot(s_disk_sync_delay_ms, [this]() { sync_dirty_domains_to_disk(); })) { diff --git a/Userland/Services/ConfigServer/ClientConnection.h b/Userland/Services/ConfigServer/ClientConnection.h index 60b8c34a3ab..5ef35bf1029 100644 --- a/Userland/Services/ConfigServer/ClientConnection.h +++ b/Userland/Services/ConfigServer/ClientConnection.h @@ -23,7 +23,7 @@ public: bool is_monitoring_domain(String const& domain) const { return m_monitored_domains.contains(domain); } private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual void pledge_domains(Vector const&) override; virtual void monitor_domain(String const&) override; diff --git a/Userland/Services/FileSystemAccessServer/ClientConnection.cpp b/Userland/Services/FileSystemAccessServer/ClientConnection.cpp index 0c045b33ba8..6ce0800d041 100644 --- a/Userland/Services/FileSystemAccessServer/ClientConnection.cpp +++ b/Userland/Services/FileSystemAccessServer/ClientConnection.cpp @@ -20,7 +20,7 @@ namespace FileSystemAccessServer { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) { s_connections.set(1, *this); @@ -72,7 +72,7 @@ void ClientConnection::request_file_handler(i32 window_server_client_id, i32 par else if (has_flag(requested_access, Core::OpenMode::WriteOnly)) access_string = "write to"; - auto pid = this->socket().peer_pid(); + auto pid = this->socket().peer_pid().release_value_but_fixme_should_propagate_errors(); auto exe_link = LexicalPath("/proc").append(String::number(pid)).append("exe").string(); auto exe_path = Core::File::real_path_for(exe_link); diff --git a/Userland/Services/FileSystemAccessServer/ClientConnection.h b/Userland/Services/FileSystemAccessServer/ClientConnection.h index ed86b40c9d2..00c1d2ef6e5 100644 --- a/Userland/Services/FileSystemAccessServer/ClientConnection.h +++ b/Userland/Services/FileSystemAccessServer/ClientConnection.h @@ -25,7 +25,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); virtual void request_file_read_only_approved(i32, i32, String const&) override; virtual void request_file(i32, i32, String const&, Core::OpenMode const&) override; diff --git a/Userland/Services/ImageDecoder/ClientConnection.cpp b/Userland/Services/ImageDecoder/ClientConnection.cpp index 6b8ec20fafb..b9199aa9d5b 100644 --- a/Userland/Services/ImageDecoder/ClientConnection.cpp +++ b/Userland/Services/ImageDecoder/ClientConnection.cpp @@ -12,7 +12,7 @@ namespace ImageDecoder { -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) { } diff --git a/Userland/Services/ImageDecoder/ClientConnection.h b/Userland/Services/ImageDecoder/ClientConnection.h index 01cb037c7ab..960627c8b48 100644 --- a/Userland/Services/ImageDecoder/ClientConnection.h +++ b/Userland/Services/ImageDecoder/ClientConnection.h @@ -25,7 +25,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); virtual Messages::ImageDecoderServer::DecodeImageResponse decode_image(Core::AnonymousBuffer const&) override; }; diff --git a/Userland/Services/InspectorServer/ClientConnection.cpp b/Userland/Services/InspectorServer/ClientConnection.cpp index f2892728822..ee5346b951f 100644 --- a/Userland/Services/InspectorServer/ClientConnection.cpp +++ b/Userland/Services/InspectorServer/ClientConnection.cpp @@ -12,7 +12,7 @@ namespace InspectorServer { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr socket, int client_id) : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/InspectorServer/ClientConnection.h b/Userland/Services/InspectorServer/ClientConnection.h index 3b27c7cd839..e90086683ec 100644 --- a/Userland/Services/InspectorServer/ClientConnection.h +++ b/Userland/Services/InspectorServer/ClientConnection.h @@ -23,7 +23,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual Messages::InspectorServer::GetAllObjectsResponse get_all_objects(pid_t) override; virtual Messages::InspectorServer::SetInspectedObjectResponse set_inspected_object(pid_t, u64 object_id) override; diff --git a/Userland/Services/InspectorServer/InspectableProcess.cpp b/Userland/Services/InspectorServer/InspectableProcess.cpp index 9e2c98f5f18..dc5d9cb01ec 100644 --- a/Userland/Services/InspectorServer/InspectableProcess.cpp +++ b/Userland/Services/InspectorServer/InspectableProcess.cpp @@ -16,14 +16,17 @@ InspectableProcess* InspectableProcess::from_pid(pid_t pid) return g_processes.get(pid).value_or(nullptr); } -InspectableProcess::InspectableProcess(pid_t pid, NonnullRefPtr socket) +InspectableProcess::InspectableProcess(pid_t pid, NonnullOwnPtr socket) : m_pid(pid) , m_socket(move(socket)) { - m_socket->set_blocking(true); + // FIXME: Propagate errors + MUST(m_socket->set_blocking(true)); + m_socket->on_ready_to_read = [this] { - [[maybe_unused]] auto buffer = m_socket->read(1); - if (m_socket->eof()) { + char c; + [[maybe_unused]] auto buffer = m_socket->read({ &c, 1 }); + if (m_socket->is_eof()) { g_processes.remove(m_pid); return; } @@ -36,46 +39,51 @@ InspectableProcess::~InspectableProcess() String InspectableProcess::wait_for_response() { - if (m_socket->eof()) { + if (m_socket->is_eof()) { dbgln("InspectableProcess disconnected: PID {}", m_pid); m_socket->close(); return {}; } u32 length {}; - auto nread = m_socket->read((u8*)&length, sizeof(length)); + auto nread = m_socket->read({ (u8*)&length, sizeof(length) }).release_value_but_fixme_should_propagate_errors(); if (nread != sizeof(length)) { dbgln("InspectableProcess got malformed data: PID {}", m_pid); m_socket->close(); return {}; } - ByteBuffer data; - size_t remaining_bytes = length; + auto data_buffer = ByteBuffer::create_uninitialized(length).release_value(); + auto remaining_data_buffer = data_buffer.bytes(); - while (remaining_bytes) { - auto packet = m_socket->read(remaining_bytes); - if (packet.size() == 0) - break; - if (auto result = data.try_append(packet.data(), packet.size()); result.is_error()) { - dbgln("Failed to append {} bytes to data buffer: {}", packet.size(), result.error()); + while (!remaining_data_buffer.is_empty()) { + auto maybe_nread = m_socket->read(remaining_data_buffer); + if (maybe_nread.is_error()) { + dbgln("InspectableProcess::wait_for_response: Failed to read data: {}", maybe_nread.error()); break; } - remaining_bytes -= packet.size(); + + auto nread = maybe_nread.release_value(); + if (nread == 0) + break; + + remaining_data_buffer = remaining_data_buffer.slice(nread); } - VERIFY(data.size() == length); + VERIFY(data_buffer.size() == length); dbgln("Got data size {} and read that many bytes", length); - return String::copy(data); + return String::copy(data_buffer); } void InspectableProcess::send_request(JsonObject const& request) { auto serialized = request.to_string(); u32 length = serialized.length(); - m_socket->write((u8 const*)&length, sizeof(length)); - m_socket->write(serialized); + + // FIXME: Propagate errors + MUST(m_socket->write({ (u8 const*)&length, sizeof(length) })); + MUST(m_socket->write(serialized.bytes())); } } diff --git a/Userland/Services/InspectorServer/InspectableProcess.h b/Userland/Services/InspectorServer/InspectableProcess.h index afefa0ba871..ed1547cce07 100644 --- a/Userland/Services/InspectorServer/InspectableProcess.h +++ b/Userland/Services/InspectorServer/InspectableProcess.h @@ -6,13 +6,13 @@ #pragma once -#include +#include namespace InspectorServer { class InspectableProcess { public: - InspectableProcess(pid_t, NonnullRefPtr); + InspectableProcess(pid_t, NonnullOwnPtr); ~InspectableProcess(); void send_request(JsonObject const& request); @@ -22,7 +22,7 @@ public: private: pid_t m_pid { 0 }; - NonnullRefPtr m_socket; + NonnullOwnPtr m_socket; }; extern HashMap> g_processes; diff --git a/Userland/Services/InspectorServer/main.cpp b/Userland/Services/InspectorServer/main.cpp index d9770f32fa9..316238ef4ce 100644 --- a/Userland/Services/InspectorServer/main.cpp +++ b/Userland/Services/InspectorServer/main.cpp @@ -25,7 +25,7 @@ ErrorOr serenity_main(Main::Arguments) TRY(inspectables_server->take_over_from_system_server("/tmp/portal/inspectables")); inspectables_server->on_accept = [&](auto client_socket) { - auto pid = client_socket->peer_pid(); + auto pid = client_socket->peer_pid().release_value_but_fixme_should_propagate_errors(); InspectorServer::g_processes.set(pid, make(pid, move(client_socket))); }; diff --git a/Userland/Services/LaunchServer/ClientConnection.cpp b/Userland/Services/LaunchServer/ClientConnection.cpp index ee9fe3f4534..5cd7dc2f9a2 100644 --- a/Userland/Services/LaunchServer/ClientConnection.cpp +++ b/Userland/Services/LaunchServer/ClientConnection.cpp @@ -13,7 +13,7 @@ namespace LaunchServer { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/LaunchServer/ClientConnection.h b/Userland/Services/LaunchServer/ClientConnection.h index 15c78dc703a..cdee50f58c3 100644 --- a/Userland/Services/LaunchServer/ClientConnection.h +++ b/Userland/Services/LaunchServer/ClientConnection.h @@ -20,7 +20,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual Messages::LaunchServer::OpenUrlResponse open_url(URL const&, String const&) override; virtual Messages::LaunchServer::GetHandlersForUrlResponse get_handlers_for_url(URL const&) override; diff --git a/Userland/Services/LookupServer/ClientConnection.cpp b/Userland/Services/LookupServer/ClientConnection.cpp index 5d937b25378..71f58d66fbc 100644 --- a/Userland/Services/LookupServer/ClientConnection.cpp +++ b/Userland/Services/LookupServer/ClientConnection.cpp @@ -13,7 +13,7 @@ namespace LookupServer { static HashMap> s_connections; -ClientConnection::ClientConnection(AK::NonnullRefPtr socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr socket, int client_id) : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/LookupServer/ClientConnection.h b/Userland/Services/LookupServer/ClientConnection.h index 60bd8697a45..a9299e2889b 100644 --- a/Userland/Services/LookupServer/ClientConnection.h +++ b/Userland/Services/LookupServer/ClientConnection.h @@ -23,7 +23,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual Messages::LookupServer::LookupNameResponse lookup_name(String const&) override; virtual Messages::LookupServer::LookupAddressResponse lookup_address(String const&) override; diff --git a/Userland/Services/NotificationServer/ClientConnection.cpp b/Userland/Services/NotificationServer/ClientConnection.cpp index c15b13a148c..394284a5b45 100644 --- a/Userland/Services/NotificationServer/ClientConnection.cpp +++ b/Userland/Services/NotificationServer/ClientConnection.cpp @@ -13,7 +13,7 @@ namespace NotificationServer { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/NotificationServer/ClientConnection.h b/Userland/Services/NotificationServer/ClientConnection.h index d35a53c99ba..b01706d8ef8 100644 --- a/Userland/Services/NotificationServer/ClientConnection.h +++ b/Userland/Services/NotificationServer/ClientConnection.h @@ -23,7 +23,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual void show_notification(String const&, String const&, Gfx::ShareableBitmap const&) override; virtual void close_notification() override; diff --git a/Userland/Services/RequestServer/ClientConnection.cpp b/Userland/Services/RequestServer/ClientConnection.cpp index 31ba16406cd..676b90a5a18 100644 --- a/Userland/Services/RequestServer/ClientConnection.cpp +++ b/Userland/Services/RequestServer/ClientConnection.cpp @@ -15,7 +15,7 @@ namespace RequestServer { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) { s_connections.set(1, *this); diff --git a/Userland/Services/RequestServer/ClientConnection.h b/Userland/Services/RequestServer/ClientConnection.h index 5a4c1ab8291..47cc38533d8 100644 --- a/Userland/Services/RequestServer/ClientConnection.h +++ b/Userland/Services/RequestServer/ClientConnection.h @@ -29,7 +29,7 @@ public: void did_request_certificates(Badge, Request&); private: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); virtual Messages::RequestServer::IsSupportedProtocolResponse is_supported_protocol(String const&) override; virtual Messages::RequestServer::StartRequestResponse start_request(String const&, URL const&, IPC::Dictionary const&, ByteBuffer const&) override; diff --git a/Userland/Services/SQLServer/ClientConnection.cpp b/Userland/Services/SQLServer/ClientConnection.cpp index d75bccf246f..d530b89ec7e 100644 --- a/Userland/Services/SQLServer/ClientConnection.cpp +++ b/Userland/Services/SQLServer/ClientConnection.cpp @@ -23,7 +23,7 @@ RefPtr ClientConnection::client_connection_for(int client_id) return nullptr; } -ClientConnection::ClientConnection(AK::NonnullRefPtr socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr socket, int client_id) : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/SQLServer/ClientConnection.h b/Userland/Services/SQLServer/ClientConnection.h index d00dca14e64..125b70e2c86 100644 --- a/Userland/Services/SQLServer/ClientConnection.h +++ b/Userland/Services/SQLServer/ClientConnection.h @@ -25,7 +25,7 @@ public: static RefPtr client_connection_for(int client_id); private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); virtual Messages::SQLServer::ConnectResponse connect(String const&) override; virtual Messages::SQLServer::SqlStatementResponse sql_statement(int, String const&) override; diff --git a/Userland/Services/SpiceAgent/ClipboardServerConnection.h b/Userland/Services/SpiceAgent/ClipboardServerConnection.h index 57c6813d40d..3e29de5ee00 100644 --- a/Userland/Services/SpiceAgent/ClipboardServerConnection.h +++ b/Userland/Services/SpiceAgent/ClipboardServerConnection.h @@ -15,7 +15,7 @@ class ClipboardServerConnection final : public IPC::ServerConnection , public ClipboardClientEndpoint { - C_OBJECT(ClipboardServerConnection); + IPC_CLIENT_CONNECTION(ClipboardServerConnection, "/tmp/portal/clipboard") public: Function on_data_changed; @@ -23,8 +23,8 @@ public: void set_bitmap(Gfx::Bitmap const& bitmap); private: - ClipboardServerConnection() - : IPC::ServerConnection(*this, "/tmp/portal/clipboard") + ClipboardServerConnection(NonnullOwnPtr socket) + : IPC::ServerConnection(*this, move(socket)) { } virtual void clipboard_data_changed(String const&) override diff --git a/Userland/Services/WebContent/ClientConnection.cpp b/Userland/Services/WebContent/ClientConnection.cpp index e17e6b52cc7..facf01ca00c 100644 --- a/Userland/Services/WebContent/ClientConnection.cpp +++ b/Userland/Services/WebContent/ClientConnection.cpp @@ -29,7 +29,7 @@ namespace WebContent { -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) , m_page_host(PageHost::create(*this)) { diff --git a/Userland/Services/WebContent/ClientConnection.h b/Userland/Services/WebContent/ClientConnection.h index ef53ee22499..833b45d4607 100644 --- a/Userland/Services/WebContent/ClientConnection.h +++ b/Userland/Services/WebContent/ClientConnection.h @@ -32,7 +32,7 @@ public: void initialize_js_console(Badge); private: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); Web::Page& page(); const Web::Page& page() const; diff --git a/Userland/Services/WebSocket/ClientConnection.cpp b/Userland/Services/WebSocket/ClientConnection.cpp index c4af86dfbd6..ba07ff5ff11 100644 --- a/Userland/Services/WebSocket/ClientConnection.cpp +++ b/Userland/Services/WebSocket/ClientConnection.cpp @@ -13,7 +13,7 @@ namespace WebSocket { static HashMap> s_connections; -ClientConnection::ClientConnection(NonnullRefPtr socket) +ClientConnection::ClientConnection(NonnullOwnPtr socket) : IPC::ClientConnection(*this, move(socket), 1) { s_connections.set(1, *this); diff --git a/Userland/Services/WebSocket/ClientConnection.h b/Userland/Services/WebSocket/ClientConnection.h index 8897ab030cd..105fd07239d 100644 --- a/Userland/Services/WebSocket/ClientConnection.h +++ b/Userland/Services/WebSocket/ClientConnection.h @@ -24,7 +24,7 @@ public: virtual void die() override; private: - explicit ClientConnection(NonnullRefPtr); + explicit ClientConnection(NonnullOwnPtr); virtual Messages::WebSocketServer::ConnectResponse connect(URL const&, String const&, Vector const&, Vector const&, IPC::Dictionary const&) override; virtual Messages::WebSocketServer::ReadyStateResponse ready_state(i32) override; diff --git a/Userland/Services/WindowServer/ClientConnection.cpp b/Userland/Services/WindowServer/ClientConnection.cpp index b697649ddc9..5152d16a990 100644 --- a/Userland/Services/WindowServer/ClientConnection.cpp +++ b/Userland/Services/WindowServer/ClientConnection.cpp @@ -45,7 +45,7 @@ ClientConnection* ClientConnection::from_client_id(int client_id) return (*it).value.ptr(); } -ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) +ClientConnection::ClientConnection(NonnullOwnPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) { if (!s_connections) diff --git a/Userland/Services/WindowServer/ClientConnection.h b/Userland/Services/WindowServer/ClientConnection.h index 9db8ed10b6e..b3632b00fff 100644 --- a/Userland/Services/WindowServer/ClientConnection.h +++ b/Userland/Services/WindowServer/ClientConnection.h @@ -81,7 +81,7 @@ public: void notify_display_link(Badge); private: - explicit ClientConnection(NonnullRefPtr, int client_id); + explicit ClientConnection(NonnullOwnPtr, int client_id); // ^ClientConnection virtual void die() override; diff --git a/Userland/Services/WindowServer/WMClientConnection.cpp b/Userland/Services/WindowServer/WMClientConnection.cpp index a17704cf26a..b6f8b7025d0 100644 --- a/Userland/Services/WindowServer/WMClientConnection.cpp +++ b/Userland/Services/WindowServer/WMClientConnection.cpp @@ -13,7 +13,7 @@ namespace WindowServer { HashMap> WMClientConnection::s_connections {}; -WMClientConnection::WMClientConnection(NonnullRefPtr client_socket, int client_id) +WMClientConnection::WMClientConnection(NonnullOwnPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) { s_connections.set(client_id, *this); diff --git a/Userland/Services/WindowServer/WMClientConnection.h b/Userland/Services/WindowServer/WMClientConnection.h index 98ae3aea255..e7f144782bd 100644 --- a/Userland/Services/WindowServer/WMClientConnection.h +++ b/Userland/Services/WindowServer/WMClientConnection.h @@ -36,7 +36,7 @@ public: int window_id() const { return m_window_id; } private: - explicit WMClientConnection(NonnullRefPtr client_socket, int client_id); + explicit WMClientConnection(NonnullOwnPtr client_socket, int client_id); // ^ClientConnection virtual void die() override; diff --git a/Userland/Utilities/aplay.cpp b/Userland/Utilities/aplay.cpp index ce8bb75a1e6..aea5998ae4f 100644 --- a/Userland/Utilities/aplay.cpp +++ b/Userland/Utilities/aplay.cpp @@ -32,7 +32,7 @@ ErrorOr serenity_main(Main::Arguments arguments) Core::EventLoop loop; - auto audio_client = Audio::ClientConnection::construct(); + auto audio_client = TRY(Audio::ClientConnection::try_create()); auto maybe_loader = Audio::Loader::create(path); if (maybe_loader.is_error()) { warnln("Failed to load audio file: {}", maybe_loader.error().description); diff --git a/Userland/Utilities/asctl.cpp b/Userland/Utilities/asctl.cpp index d4214cf858c..8e9377e2788 100644 --- a/Userland/Utilities/asctl.cpp +++ b/Userland/Utilities/asctl.cpp @@ -29,7 +29,7 @@ enum AudioVariable : u32 { ErrorOr serenity_main(Main::Arguments arguments) { Core::EventLoop loop; - auto audio_client = Audio::ClientConnection::construct(); + auto audio_client = TRY(Audio::ClientConnection::try_create()); String command = String::empty(); Vector command_arguments; diff --git a/Userland/Utilities/pro.cpp b/Userland/Utilities/pro.cpp index aad667287e5..7307699fc3e 100644 --- a/Userland/Utilities/pro.cpp +++ b/Userland/Utilities/pro.cpp @@ -188,7 +188,7 @@ ErrorOr serenity_main(Main::Arguments arguments) } Core::EventLoop loop; - auto protocol_client = Protocol::RequestClient::construct(); + auto protocol_client = TRY(Protocol::RequestClient::try_create()); auto request = protocol_client->start_request(method, url, request_headers, data ? StringView { data }.bytes() : ReadonlyBytes {}); if (!request) { diff --git a/Userland/Utilities/sql.cpp b/Userland/Utilities/sql.cpp index b078b728bde..120144cef02 100644 --- a/Userland/Utilities/sql.cpp +++ b/Userland/Utilities/sql.cpp @@ -71,7 +71,7 @@ public: m_editor->set_prompt(prompt_for_level(open_indents)); }; - m_sql_client = SQL::SQLClient::construct(); + m_sql_client = SQL::SQLClient::try_create().release_value_but_fixme_should_propagate_errors(); m_sql_client->on_connected = [this](int connection_id, String const& connected_to_database) { outln("Connected to \033[33;1m{}\033[0m", connected_to_database); diff --git a/Userland/Utilities/telws.cpp b/Userland/Utilities/telws.cpp index ff0a9e1e318..502b13fc8b8 100644 --- a/Userland/Utilities/telws.cpp +++ b/Userland/Utilities/telws.cpp @@ -40,9 +40,15 @@ int main(int argc, char** argv) } Core::EventLoop loop; + + auto maybe_websocket_client = Protocol::WebSocketClient::try_create(); + if (maybe_websocket_client.is_error()) { + warnln("Failed to connect to the websocket server: {}\n", maybe_websocket_client.error()); + } + auto websocket_client = maybe_websocket_client.release_value(); + RefPtr editor = Line::Editor::construct(); bool should_quit = false; - auto websocket_client = Protocol::WebSocketClient::construct(); auto socket = websocket_client->connect(url, origin); if (!socket) { warnln("Failed to start socket for '{}'\n", url);