LibCore+Inspector: Reverse the direction of Inspector connections

Core::EventLoop now makes an outbound connection to InspectorServer
instead of listening for incoming connections on a /tmp/rpc/PID socket.

This has many benefits, for example:
- We no longer keep an open listening socket in most applications
- We stop leaking socket files in /tmp/rpc
- We can tighten the pledges in many programs (patch coming)

Userland/DevTools/Inspector/RemoteProcess.cpp

@@ -8,11 +8,37 @@
 #include "RemoteObject.h"
 #include "RemoteObjectGraphModel.h"
 #include "RemoteObjectPropertyModel.h"
+#include <InspectorServer/InspectorClientEndpoint.h>
+#include <InspectorServer/InspectorServerEndpoint.h>
+#include <LibIPC/ServerConnection.h>
 #include <stdio.h>
 #include <stdlib.h>
 
 namespace Inspector {
 
+class InspectorServerClient final
+    : public IPC::ServerConnection<InspectorClientEndpoint, InspectorServerEndpoint>
+    , public InspectorClientEndpoint {
+    C_OBJECT(InspectorServerClient);
+
+public:
+    virtual void handshake() override
+    {
+        greet();
+    }
+
+    virtual ~InspectorServerClient() override { }
+
+private:
+    InspectorServerClient()
+        : IPC::ServerConnection<InspectorClientEndpoint, InspectorServerEndpoint>(*this, "/tmp/portal/inspector")
+    {
+        handshake();
+    }
+
+    virtual void dummy() override { }
+};
+
 RemoteProcess* s_the;
 
 RemoteProcess& RemoteProcess::the()
@@ -23,10 +49,13 @@ RemoteProcess& RemoteProcess::the()
 RemoteProcess::RemoteProcess(pid_t pid)
     : m_pid(pid)
     , m_object_graph_model(RemoteObjectGraphModel::create(*this))
-    , m_socket(Core::LocalSocket::construct())
 {
     s_the = this;
-    m_socket->set_blocking(true);
+    m_client = InspectorServerClient::construct();
+}
+
+RemoteProcess::~RemoteProcess()
+{
 }
 
 void RemoteProcess::handle_identify_response(const JsonObject& response)
@@ -79,106 +108,28 @@ void RemoteProcess::handle_get_all_objects_response(const JsonObject& response)
         on_update();
 }
 
-void RemoteProcess::send_request(const JsonObject& request)
-{
-    auto serialized = request.to_string();
-    i32 length = serialized.length();
-    m_socket->write((const u8*)&length, sizeof(length));
-    m_socket->write(serialized);
-}
-
 void RemoteProcess::set_inspected_object(FlatPtr address)
 {
-    JsonObject request;
-    request.set("type", "SetInspectedObject");
-    request.set("address", address);
-    send_request(request);
+    m_client->async_set_inspected_object(m_pid, address);
 }
 
 void RemoteProcess::set_property(FlatPtr object, const StringView& name, const JsonValue& value)
 {
-    JsonObject request;
-    request.set("type", "SetProperty");
-    request.set("address", object);
-    request.set("name", JsonValue(name));
-    request.set("value", value);
-    send_request(request);
+    m_client->async_set_object_property(m_pid, object, name, value.to_string());
 }
 
 void RemoteProcess::update()
 {
-    m_socket->on_connected = [this] {
-        dbgln("Connected to PID {}", m_pid);
-
     {
-            JsonObject request;
-            request.set("type", "Identify");
-            send_request(request);
+        auto raw_json = m_client->identify(m_pid);
+        auto json = JsonValue::from_string(raw_json);
+        handle_identify_response(json.value().as_object());
     }
 
     {
-            JsonObject request;
-            request.set("type", "GetAllObjects");
-            send_request(request);
-        }
-    };
-
-    m_socket->on_ready_to_read = [this] {
-        if (m_socket->eof()) {
-            dbgln("Disconnected from PID {}", m_pid);
-            m_socket->close();
-            return;
-        }
-
-        u32 length;
-        int nread = m_socket->read((u8*)&length, sizeof(length));
-        if (nread != sizeof(length)) {
-            dbgln("Disconnected from PID {}", m_pid);
-            m_socket->close();
-            return;
-        }
-
-        ByteBuffer data;
-        size_t remaining_bytes = length;
-
-        while (remaining_bytes) {
-            auto packet = m_socket->read(remaining_bytes);
-            if (packet.size() == 0)
-                break;
-            data.append(packet.data(), packet.size());
-            remaining_bytes -= packet.size();
-        }
-
-        VERIFY(data.size() == length);
-        dbgln("Got data size {} and read that many bytes", length);
-
-        auto json_value = JsonValue::from_string(data);
-        VERIFY(json_value.has_value());
-        VERIFY(json_value.value().is_object());
-
-        dbgln("Got JSON response {}", json_value.value());
-
-        auto& response = json_value.value().as_object();
-
-        auto response_type = response.get("type").as_string_or({});
-        if (response_type.is_null())
-            return;
-
-        if (response_type == "GetAllObjects") {
-            handle_get_all_objects_response(response);
-            return;
-        }
-
-        if (response_type == "Identify") {
-            handle_identify_response(response);
-            return;
-        }
-    };
-
-    auto success = m_socket->connect(Core::SocketAddress::local(String::formatted("/tmp/rpc/{}", m_pid)));
-    if (!success) {
-        warnln("Couldn't connect to PID {}", m_pid);
-        exit(1);
+        auto raw_json = m_client->get_all_objects(m_pid);
+        auto json = JsonValue::from_string(raw_json);
+        handle_get_all_objects_response(json.value().as_object());
     }
 }
 

Userland/DevTools/Inspector/RemoteProcess.h

@@ -11,6 +11,7 @@
 
 namespace Inspector {
 
+class InspectorServerClient;
 class RemoteObjectGraphModel;
 class RemoteObject;
 
@@ -19,6 +20,7 @@ public:
     static RemoteProcess& the();
 
     explicit RemoteProcess(pid_t);
+    ~RemoteProcess();
     void update();
 
     pid_t pid() const { return m_pid; }
@@ -42,8 +44,8 @@ private:
     pid_t m_pid { -1 };
     String m_process_name;
     NonnullRefPtr<RemoteObjectGraphModel> m_object_graph_model;
-    RefPtr<Core::LocalSocket> m_socket;
     NonnullOwnPtrVector<RemoteObject> m_roots;
+    RefPtr<InspectorServerClient> m_client;
 };
 
 }

Userland/Libraries/LibCore/EventLoop.cpp

@@ -37,7 +37,9 @@
 
 namespace Core {
 
-class RPCClient;
+class InspectorServerConnection;
+
+[[maybe_unused]] static bool connect_to_inspector_server();
 
 struct EventLoopTimer {
     int timer_id { 0 };
@@ -61,8 +63,7 @@ static NeverDestroyed<IDAllocator> s_id_allocator;
 static HashMap<int, NonnullOwnPtr<EventLoopTimer>>* s_timers;
 static HashTable<Notifier*>* s_notifiers;
 int EventLoop::s_wake_pipe_fds[2];
-static RefPtr<LocalServer> s_rpc_server;
-HashMap<int, RefPtr<RPCClient>> s_rpc_clients;
+static RefPtr<InspectorServerConnection> s_inspector_server_connection;
 
 class SignalHandlers : public RefCounted<SignalHandlers> {
     AK_MAKE_NONCOPYABLE(SignalHandlers);
@@ -120,15 +121,14 @@ inline SignalHandlersInfo* signals_info()
 
 pid_t EventLoop::s_pid;
 
-class RPCClient : public Object {
-    C_OBJECT(RPCClient)
+class InspectorServerConnection : public Object {
+    C_OBJECT(InspectorServerConnection)
 public:
-    explicit RPCClient(RefPtr<LocalSocket> socket)
+    explicit InspectorServerConnection(RefPtr<LocalSocket> socket)
         : m_socket(move(socket))
         , m_client_id(s_id_allocator->allocate())
     {
 #ifdef __serenity__
-        s_rpc_clients.set(m_client_id, this);
         add_child(*m_socket);
         m_socket->on_ready_to_read = [this] {
             u32 length;
@@ -154,7 +154,7 @@ public:
         warnln("RPC Client constructed outside serenity, this is very likely a bug!");
 #endif
     }
-    virtual ~RPCClient() override
+    virtual ~InspectorServerConnection() override
     {
         if (auto inspected_object = m_inspected_object.strong_ref())
             inspected_object->decrement_inspector_count({});
@@ -244,7 +244,6 @@ public:
 
     void shutdown()
     {
-        s_rpc_clients.remove(m_client_id);
         s_id_allocator->deallocate(m_client_id);
     }
 
@@ -254,7 +253,7 @@ private:
     int m_client_id { -1 };
 };
 
-EventLoop::EventLoop()
+EventLoop::EventLoop([[maybe_unused]] MakeInspectable make_inspectable)
     : m_private(make<Private>())
 {
     if (!s_event_loop_stack) {
@@ -278,9 +277,11 @@ EventLoop::EventLoop()
         s_event_loop_stack->append(this);
 
 #ifdef __serenity__
-        if (!s_rpc_server) {
-            if (!start_rpc_server())
-                dbgln("Core::EventLoop: Failed to start an RPC server");
+        if (make_inspectable == MakeInspectable::Yes) {
+            if (!s_inspector_server_connection) {
+                if (!connect_to_inspector_server())
+                    dbgln("Core::EventLoop: Failed to connect to InspectorServer");
+            }
         }
 #endif
     }
@@ -292,15 +293,14 @@ EventLoop::~EventLoop()
 {
 }
 
-bool EventLoop::start_rpc_server()
+bool connect_to_inspector_server()
 {
 #ifdef __serenity__
-    s_rpc_server = LocalServer::construct();
-    s_rpc_server->set_name("Core::EventLoop_RPC_server");
-    s_rpc_server->on_ready_to_accept = [&] {
-        RPCClient::construct(s_rpc_server->accept());
-    };
-    return s_rpc_server->listen(String::formatted("/tmp/rpc/{}", getpid()));
+    auto socket = Core::LocalSocket::construct();
+    if (!socket->connect(SocketAddress::local("/tmp/portal/inspectables")))
+        return false;
+    s_inspector_server_connection = InspectorServerConnection::construct(move(socket));
+    return true;
 #else
     VERIFY_NOT_REACHED();
 #endif
@@ -563,8 +563,7 @@ void EventLoop::notify_forked(ForkEvent event)
         }
         s_pid = 0;
 #ifdef __serenity__
-        s_rpc_server = nullptr;
-        s_rpc_clients.clear();
+        s_inspector_server_connection = nullptr;
 #endif
         return;
     }

Userland/Libraries/LibCore/EventLoop.h

@@ -21,7 +21,12 @@ namespace Core {
 
 class EventLoop {
 public:
-    EventLoop();
+    enum class MakeInspectable {
+        No,
+        Yes,
+    };
+
+    EventLoop(MakeInspectable = MakeInspectable::Yes);
     ~EventLoop();
 
     int exec();
@@ -69,7 +74,6 @@ public:
     static void notify_forked(ForkEvent);
 
 private:
-    bool start_rpc_server();
     void wait_for_event(WaitMode);
     Optional<struct timeval> get_next_timer_expiration();
     static void dispatch_signal(int);

Userland/Libraries/LibCore/Object.cpp

@@ -226,14 +226,14 @@ bool Object::is_visible_for_timer_purposes() const
     return true;
 }
 
-void Object::increment_inspector_count(Badge<RPCClient>)
+void Object::increment_inspector_count(Badge<InspectorServerConnection>)
 {
     ++m_inspector_count;
     if (m_inspector_count == 1)
         did_begin_inspection();
 }
 
-void Object::decrement_inspector_count(Badge<RPCClient>)
+void Object::decrement_inspector_count(Badge<InspectorServerConnection>)
 {
     --m_inspector_count;
     if (!m_inspector_count)

Userland/Libraries/LibCore/Object.h

@@ -48,7 +48,7 @@ private:
     ObjectClassRegistration* m_parent_class { nullptr };
 };
 
-class RPCClient;
+class InspectorServerConnection;
 
 enum class TimerShouldFireWhenNotVisible {
     No = 0,
@@ -155,8 +155,8 @@ public:
 
     bool is_being_inspected() const { return m_inspector_count; }
 
-    void increment_inspector_count(Badge<RPCClient>);
-    void decrement_inspector_count(Badge<RPCClient>);
+    void increment_inspector_count(Badge<InspectorServerConnection>);
+    void decrement_inspector_count(Badge<InspectorServerConnection>);
 
     virtual bool load_from_json(const JsonObject&, RefPtr<Core::Object> (*)(const String&)) { return false; }