From 94ddb07e5884675d96a6938069fc3e83698dd898 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 6 Jul 2020 13:23:39 +0200 Subject: [PATCH] LibIPC+Services: Make ClientConnection take socket as NonnullRefPtr This avoids getting into the awkward situation where the socket is still part-owned by main() in multi-instance service. Also it just reads nicer. --- Libraries/LibIPC/ClientConnection.h | 15 +++++++-------- Services/AudioServer/ClientConnection.cpp | 4 ++-- Services/AudioServer/ClientConnection.h | 2 +- Services/AudioServer/main.cpp | 2 +- Services/Clipboard/ClientConnection.cpp | 4 ++-- Services/Clipboard/ClientConnection.h | 2 +- Services/Clipboard/main.cpp | 2 +- Services/ImageDecoder/CMakeLists.txt | 2 +- Services/ImageDecoder/ClientConnection.cpp | 4 ++-- Services/ImageDecoder/ClientConnection.h | 2 +- Services/ImageDecoder/main.cpp | 2 +- Services/LaunchServer/ClientConnection.cpp | 4 ++-- Services/LaunchServer/ClientConnection.h | 2 +- Services/LaunchServer/main.cpp | 2 +- Services/NotificationServer/ClientConnection.cpp | 4 ++-- Services/NotificationServer/ClientConnection.h | 2 +- Services/NotificationServer/main.cpp | 2 +- Services/ProtocolServer/ClientConnection.cpp | 4 ++-- Services/ProtocolServer/ClientConnection.h | 2 +- Services/WebContent/ClientConnection.cpp | 4 ++-- Services/WebContent/ClientConnection.h | 2 +- Services/WebContent/main.cpp | 3 ++- Services/WindowServer/ClientConnection.cpp | 4 ++-- Services/WindowServer/ClientConnection.h | 2 +- Services/WindowServer/EventLoop.cpp | 2 +- 25 files changed, 40 insertions(+), 40 deletions(-) diff --git a/Libraries/LibIPC/ClientConnection.h b/Libraries/LibIPC/ClientConnection.h index 78b6b6c3d8..0e2da07bf9 100644 --- a/Libraries/LibIPC/ClientConnection.h +++ b/Libraries/LibIPC/ClientConnection.h @@ -48,7 +48,7 @@ public: Invalid = 2000, Disconnected, }; - Event() {} + Event() { } explicit Event(Type type) : Core::Event(type) { @@ -78,19 +78,18 @@ NonnullRefPtr new_client_connection(Args&&... args) template class ClientConnection : public Core::Object { public: - ClientConnection(Endpoint& endpoint, Core::LocalSocket& socket, int client_id) + ClientConnection(Endpoint& endpoint, NonnullRefPtr socket, int client_id) : m_endpoint(endpoint) - , m_socket(socket) + , m_socket(move(socket)) , m_client_id(client_id) { - ASSERT(socket.is_connected()); + ASSERT(m_socket->is_connected()); ucred creds; socklen_t creds_size = sizeof(creds); if (getsockopt(m_socket->fd(), SOL_SOCKET, SO_PEERCRED, &creds, &creds_size) < 0) { ASSERT_NOT_REACHED(); } m_client_pid = creds.pid; - add_child(socket); m_socket->on_ready_to_read = [this] { drain_messages_from_client(); }; m_responsiveness_timer = Core::Timer::create_single_shot(3000, [this] { may_have_become_unresponsive(); }); @@ -100,8 +99,8 @@ public: { } - virtual void may_have_become_unresponsive() {} - virtual void did_become_responsive() {} + virtual void may_have_become_unresponsive() { } + virtual void did_become_responsive() { } void post_message(const Message& message) { @@ -223,7 +222,7 @@ protected: private: Endpoint& m_endpoint; - RefPtr m_socket; + NonnullRefPtr m_socket; RefPtr m_responsiveness_timer; int m_client_id { -1 }; int m_client_pid { -1 }; diff --git a/Services/AudioServer/ClientConnection.cpp b/Services/AudioServer/ClientConnection.cpp index a7d5a3d540..117778b417 100644 --- a/Services/AudioServer/ClientConnection.cpp +++ b/Services/AudioServer/ClientConnection.cpp @@ -50,8 +50,8 @@ void ClientConnection::for_each(Function callback) callback(connection); } -ClientConnection::ClientConnection(Core::LocalSocket& client_socket, int client_id, Mixer& mixer) - : IPC::ClientConnection(*this, client_socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id, Mixer& mixer) + : IPC::ClientConnection(*this, move(client_socket), client_id) , m_mixer(mixer) { s_connections.set(client_id, *this); diff --git a/Services/AudioServer/ClientConnection.h b/Services/AudioServer/ClientConnection.h index 2befe48ab8..9460cd7ca7 100644 --- a/Services/AudioServer/ClientConnection.h +++ b/Services/AudioServer/ClientConnection.h @@ -43,7 +43,7 @@ class ClientConnection final : public IPC::ClientConnection , public AudioServerEndpoint { C_OBJECT(ClientConnection) public: - explicit ClientConnection(Core::LocalSocket&, int client_id, Mixer& mixer); + explicit ClientConnection(NonnullRefPtr, int client_id, Mixer& mixer); ~ClientConnection() override; void did_finish_playing_buffer(Badge, int buffer_id); diff --git a/Services/AudioServer/main.cpp b/Services/AudioServer/main.cpp index 6f960396cc..d529bf7d06 100644 --- a/Services/AudioServer/main.cpp +++ b/Services/AudioServer/main.cpp @@ -49,7 +49,7 @@ int main(int, char**) } static int s_next_client_id = 0; int client_id = ++s_next_client_id; - IPC::new_client_connection(*client_socket, client_id, mixer); + IPC::new_client_connection(client_socket.release_nonnull(), client_id, mixer); }; if (pledge("stdio thread shared_buffer accept", nullptr) < 0) { diff --git a/Services/Clipboard/ClientConnection.cpp b/Services/Clipboard/ClientConnection.cpp index 16b882d1e9..905a8f65b4 100644 --- a/Services/Clipboard/ClientConnection.cpp +++ b/Services/Clipboard/ClientConnection.cpp @@ -41,8 +41,8 @@ void ClientConnection::for_each_client(Function callbac } } -ClientConnection::ClientConnection(Core::LocalSocket& socket, int client_id) - : IPC::ClientConnection(*this, socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) + : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); } diff --git a/Services/Clipboard/ClientConnection.h b/Services/Clipboard/ClientConnection.h index 904b04abc1..857d749c29 100644 --- a/Services/Clipboard/ClientConnection.h +++ b/Services/Clipboard/ClientConnection.h @@ -37,7 +37,7 @@ class ClientConnection final : public IPC::ClientConnection, int client_id); virtual ~ClientConnection() override; virtual void die() override; diff --git a/Services/Clipboard/main.cpp b/Services/Clipboard/main.cpp index 18839418b8..c46c1944eb 100644 --- a/Services/Clipboard/main.cpp +++ b/Services/Clipboard/main.cpp @@ -63,7 +63,7 @@ int main(int, char**) } static int s_next_client_id = 0; int client_id = ++s_next_client_id; - IPC::new_client_connection(*client_socket, client_id); + IPC::new_client_connection(client_socket.release_nonnull(), client_id); }; Clipboard::Storage::the().on_content_change = [&] { diff --git a/Services/ImageDecoder/CMakeLists.txt b/Services/ImageDecoder/CMakeLists.txt index ce9ad077fc..56482fe31d 100644 --- a/Services/ImageDecoder/CMakeLists.txt +++ b/Services/ImageDecoder/CMakeLists.txt @@ -9,4 +9,4 @@ set(SOURCES ) serenity_bin(ImageDecoder) -target_link_libraries(ImageDecoder LibIPC LibGfx) +target_link_libraries(ImageDecoder LibGfx LibIPC) diff --git a/Services/ImageDecoder/ClientConnection.cpp b/Services/ImageDecoder/ClientConnection.cpp index ce7ec85d6b..cea63ce8d9 100644 --- a/Services/ImageDecoder/ClientConnection.cpp +++ b/Services/ImageDecoder/ClientConnection.cpp @@ -36,8 +36,8 @@ namespace ImageDecoder { static HashMap> s_connections; -ClientConnection::ClientConnection(Core::LocalSocket& socket, int client_id) - : IPC::ClientConnection(*this, socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) + : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); } diff --git a/Services/ImageDecoder/ClientConnection.h b/Services/ImageDecoder/ClientConnection.h index 3b47ed1894..2021d73682 100644 --- a/Services/ImageDecoder/ClientConnection.h +++ b/Services/ImageDecoder/ClientConnection.h @@ -40,7 +40,7 @@ class ClientConnection final C_OBJECT(ClientConnection); public: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); ~ClientConnection() override; virtual void die() override; diff --git a/Services/ImageDecoder/main.cpp b/Services/ImageDecoder/main.cpp index 1ee15b1496..d41a97411e 100644 --- a/Services/ImageDecoder/main.cpp +++ b/Services/ImageDecoder/main.cpp @@ -42,7 +42,7 @@ int main(int, char**) } auto socket = Core::LocalSocket::take_over_accepted_socket_from_system_server(); - IPC::new_client_connection(*socket, 1); + IPC::new_client_connection(socket.release_nonnull(), 1); if (pledge("stdio shared_buffer", nullptr) < 0) { perror("pledge"); return 1; diff --git a/Services/LaunchServer/ClientConnection.cpp b/Services/LaunchServer/ClientConnection.cpp index 168d6a6727..533ae8cbff 100644 --- a/Services/LaunchServer/ClientConnection.cpp +++ b/Services/LaunchServer/ClientConnection.cpp @@ -33,8 +33,8 @@ namespace LaunchServer { static HashMap> s_connections; -ClientConnection::ClientConnection(Core::LocalSocket& client_socket, int client_id) - : IPC::ClientConnection(*this, client_socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) + : IPC::ClientConnection(*this, move(client_socket), client_id) { s_connections.set(client_id, *this); } diff --git a/Services/LaunchServer/ClientConnection.h b/Services/LaunchServer/ClientConnection.h index 0d1a819f78..79019a86e8 100644 --- a/Services/LaunchServer/ClientConnection.h +++ b/Services/LaunchServer/ClientConnection.h @@ -40,7 +40,7 @@ public: virtual void die() override; private: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); virtual OwnPtr handle(const Messages::LaunchServer::Greet&) override; virtual OwnPtr handle(const Messages::LaunchServer::OpenURL&) override; diff --git a/Services/LaunchServer/main.cpp b/Services/LaunchServer/main.cpp index 97c83fcbfb..f589856d9c 100644 --- a/Services/LaunchServer/main.cpp +++ b/Services/LaunchServer/main.cpp @@ -61,7 +61,7 @@ int main(int argc, char** argv) static int s_next_client_id = 0; int client_id = ++s_next_client_id; dbg() << "Received connection"; - IPC::new_client_connection(*client_socket, client_id); + IPC::new_client_connection(client_socket.release_nonnull(), client_id); }; return event_loop.exec(); diff --git a/Services/NotificationServer/ClientConnection.cpp b/Services/NotificationServer/ClientConnection.cpp index 886f608624..f90878efe2 100644 --- a/Services/NotificationServer/ClientConnection.cpp +++ b/Services/NotificationServer/ClientConnection.cpp @@ -33,8 +33,8 @@ namespace NotificationServer { static HashMap> s_connections; -ClientConnection::ClientConnection(Core::LocalSocket& client_socket, int client_id) - : IPC::ClientConnection(*this, client_socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) + : IPC::ClientConnection(*this, move(client_socket), client_id) { s_connections.set(client_id, *this); } diff --git a/Services/NotificationServer/ClientConnection.h b/Services/NotificationServer/ClientConnection.h index bec07fb127..8ea04bbfff 100644 --- a/Services/NotificationServer/ClientConnection.h +++ b/Services/NotificationServer/ClientConnection.h @@ -40,7 +40,7 @@ public: virtual void die() override; private: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); virtual OwnPtr handle(const Messages::NotificationServer::Greet&) override; virtual OwnPtr handle(const Messages::NotificationServer::ShowNotification&) override; diff --git a/Services/NotificationServer/main.cpp b/Services/NotificationServer/main.cpp index 1eb2152886..59fe6671be 100644 --- a/Services/NotificationServer/main.cpp +++ b/Services/NotificationServer/main.cpp @@ -52,7 +52,7 @@ int main(int argc, char** argv) } static int s_next_client_id = 0; int client_id = ++s_next_client_id; - IPC::new_client_connection(*client_socket, client_id); + IPC::new_client_connection(client_socket.release_nonnull(), client_id); }; if (unveil("/res", "r") < 0) { diff --git a/Services/ProtocolServer/ClientConnection.cpp b/Services/ProtocolServer/ClientConnection.cpp index 21c23d3ec8..2b72ef0025 100644 --- a/Services/ProtocolServer/ClientConnection.cpp +++ b/Services/ProtocolServer/ClientConnection.cpp @@ -35,8 +35,8 @@ namespace ProtocolServer { static HashMap> s_connections; -ClientConnection::ClientConnection(Core::LocalSocket& socket, int client_id) - : IPC::ClientConnection(*this, socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) + : IPC::ClientConnection(*this, move(socket), client_id) { s_connections.set(client_id, *this); } diff --git a/Services/ProtocolServer/ClientConnection.h b/Services/ProtocolServer/ClientConnection.h index cb0ca45b23..bedba4801f 100644 --- a/Services/ProtocolServer/ClientConnection.h +++ b/Services/ProtocolServer/ClientConnection.h @@ -39,7 +39,7 @@ class ClientConnection final C_OBJECT(ClientConnection); public: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); ~ClientConnection() override; virtual void die() override; diff --git a/Services/WebContent/ClientConnection.cpp b/Services/WebContent/ClientConnection.cpp index 325b51b421..9def52c455 100644 --- a/Services/WebContent/ClientConnection.cpp +++ b/Services/WebContent/ClientConnection.cpp @@ -36,8 +36,8 @@ namespace WebContent { static HashMap> s_connections; -ClientConnection::ClientConnection(Core::LocalSocket& socket, int client_id) - : IPC::ClientConnection(*this, socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr socket, int client_id) + : IPC::ClientConnection(*this, move(socket), client_id) , m_page_host(PageHost::create(*this)) { s_connections.set(client_id, *this); diff --git a/Services/WebContent/ClientConnection.h b/Services/WebContent/ClientConnection.h index b2981e6be5..8609bb8e62 100644 --- a/Services/WebContent/ClientConnection.h +++ b/Services/WebContent/ClientConnection.h @@ -40,7 +40,7 @@ class ClientConnection final C_OBJECT(ClientConnection); public: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); ~ClientConnection() override; virtual void die() override; diff --git a/Services/WebContent/main.cpp b/Services/WebContent/main.cpp index cf9b9d6771..5f5bf52635 100644 --- a/Services/WebContent/main.cpp +++ b/Services/WebContent/main.cpp @@ -54,6 +54,7 @@ int main(int, char**) } auto socket = Core::LocalSocket::take_over_accepted_socket_from_system_server(); - IPC::new_client_connection(*socket, 1); + ASSERT(socket); + IPC::new_client_connection(socket.release_nonnull(), 1); return event_loop.exec(); } diff --git a/Services/WindowServer/ClientConnection.cpp b/Services/WindowServer/ClientConnection.cpp index ffada938af..2eb3b5aaa2 100644 --- a/Services/WindowServer/ClientConnection.cpp +++ b/Services/WindowServer/ClientConnection.cpp @@ -77,8 +77,8 @@ ClientConnection* ClientConnection::from_client_id(int client_id) return (*it).value.ptr(); } -ClientConnection::ClientConnection(Core::LocalSocket& client_socket, int client_id) - : IPC::ClientConnection(*this, client_socket, client_id) +ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) + : IPC::ClientConnection(*this, move(client_socket), client_id) { if (!s_connections) s_connections = new HashMap>; diff --git a/Services/WindowServer/ClientConnection.h b/Services/WindowServer/ClientConnection.h index 0ca52c1531..331dd79327 100644 --- a/Services/WindowServer/ClientConnection.h +++ b/Services/WindowServer/ClientConnection.h @@ -84,7 +84,7 @@ public: void notify_display_link(Badge); private: - explicit ClientConnection(Core::LocalSocket&, int client_id); + explicit ClientConnection(NonnullRefPtr, int client_id); // ^ClientConnection virtual void die() override; diff --git a/Services/WindowServer/EventLoop.cpp b/Services/WindowServer/EventLoop.cpp index 6d99985452..0f300935a1 100644 --- a/Services/WindowServer/EventLoop.cpp +++ b/Services/WindowServer/EventLoop.cpp @@ -63,7 +63,7 @@ EventLoop::EventLoop() } static int s_next_client_id = 0; int client_id = ++s_next_client_id; - IPC::new_client_connection(*client_socket, client_id); + IPC::new_client_connection(client_socket.release_nonnull(), client_id); }; ASSERT(m_keyboard_fd >= 0);