LibIPC: Close received IPC::File fd's by default unless taken

When receiving a file descriptor over IPC, the receiver must now call
take_fd() on the IPC::File to take over the descriptor. Otherwise,
IPC::File will close the file on destruction.

5 files changed, 50 insertions, 5 deletions

diff --git a/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.cpp b/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.cpp
index 7952696b60..c8194636f4 100644
--- a/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.cpp
+++ b/Userland/DevTools/HackStudio/LanguageServers/Cpp/ClientConnection.cpp
@@ -78,7 +78,7 @@ static DefaultDocumentClient s_default_document_client;
 void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message)
 {
     auto file = Core::File::construct(this);
-    if (!file->open(message.file().fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) {
+    if (!file->open(message.file().take_fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) {
         errno = file->error();
         perror("open");
         dbgln("Failed to open project file");
diff --git a/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.cpp b/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.cpp
index 794085f618..ef85646868 100644
--- a/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.cpp
+++ b/Userland/DevTools/HackStudio/LanguageServers/Shell/ClientConnection.cpp
@@ -78,7 +78,7 @@ static DefaultDocumentClient s_default_document_client;
 void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message)
 {
     auto file = Core::File::construct(this);
-    if (!file->open(message.file().fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) {
+    if (!file->open(message.file().take_fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) {
         errno = file->error();
         perror("open");
         dbgln("Failed to open project file");
diff --git a/Userland/Libraries/LibIPC/Decoder.cpp b/Userland/Libraries/LibIPC/Decoder.cpp
index ff980bdaa5..96871f0b5b 100644
--- a/Userland/Libraries/LibIPC/Decoder.cpp
+++ b/Userland/Libraries/LibIPC/Decoder.cpp
@@ -174,7 +174,7 @@ bool Decoder::decode([[maybe_unused]] File& file)
         dbgln("recvfd: {}", strerror(errno));
         return false;
     }
-    file = File(fd);
+    file = File(fd, File::ConstructWithReceivedFileDescriptor);
     return true;
 #else
     [[maybe_unused]] auto fd = m_sockfd;
diff --git a/Userland/Libraries/LibIPC/File.h b/Userland/Libraries/LibIPC/File.h
index 85c1137ad6..f18d6a6f15 100644
--- a/Userland/Libraries/LibIPC/File.h
+++ b/Userland/Libraries/LibIPC/File.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Sergey Bugaev <bugaevc@serenityos.org>
+ * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -26,9 +27,15 @@
 
 #pragma once
 
+#include <AK/Noncopyable.h>
+#include <AK/StdLibExtras.h>
+#include <unistd.h>
+
 namespace IPC {
 
 class File {
+    AK_MAKE_NONCOPYABLE(File);
+
 public:
     // Must have a default constructor, because LibIPC
     // default-constructs arguments prior to decoding them.
@@ -40,10 +47,48 @@ public:
     {
     }
 
+    // Tagged constructor for fd's that should be closed on destruction unless take_fd() is called.
+    enum Tag {
+        ConstructWithReceivedFileDescriptor = 1,
+    };
+    File(int fd, Tag)
+        : m_fd(fd)
+        , m_close_on_destruction(true)
+    {
+    }
+
+    File(File&& other)
+        : m_fd(exchange(other.m_fd, -1))
+        , m_close_on_destruction(exchange(other.m_close_on_destruction, false))
+    {
+    }
+
+    File& operator=(File&& other)
+    {
+        if (this != &other) {
+            m_fd = exchange(other.m_fd, -1);
+            m_close_on_destruction = exchange(other.m_close_on_destruction, false);
+        }
+        return *this;
+    }
+
+    ~File()
+    {
+        if (m_close_on_destruction && m_fd != -1)
+            close(m_fd);
+    }
+
     int fd() const { return m_fd; }
 
+    // NOTE: This is 'const' since generated IPC messages expose all parameters by const reference.
+    [[nodiscard]] int take_fd() const
+    {
+        return exchange(m_fd, -1);
+    }
+
 private:
-    int m_fd { -1 };
+    mutable int m_fd { -1 };
+    bool m_close_on_destruction { false };
 };
 
 }
diff --git a/Userland/Libraries/LibProtocol/Client.cpp b/Userland/Libraries/LibProtocol/Client.cpp
index 6d227fe1b1..4218aa53ec 100644
--- a/Userland/Libraries/LibProtocol/Client.cpp
+++ b/Userland/Libraries/LibProtocol/Client.cpp
@@ -59,7 +59,7 @@ RefPtr<Download> Client::start_download(const String& method, const String& url,
     auto download_id = response->download_id();
     if (download_id < 0 || !response->response_fd().has_value())
         return nullptr;
-    auto response_fd = response->response_fd().value().fd();
+    auto response_fd = response->response_fd().value().take_fd();
     auto download = Download::create_from_id({}, *this, download_id);
     download->set_download_fd({}, response_fd);
     m_downloads.set(download_id, download);