LibIPC+Userland: Make IPC::File always own its file descriptor

Add factory functions to distinguish between when the owner of the File
wants to transfer ownership to the new IPC object (adopt) or to send a
copy of the same fd to the IPC peer (clone).

This behavior is more intuitive than the previous behavior. Previously,
an IPC::File would default to a shallow clone of the file descriptor,
only *actually* calling dup(2) for the fd when encoding or it into an
IPC MessageBuffer. Now the dup(2) for the fd is explicit in the clone_fd
factory function.

Userland/DevTools/HackStudio/LanguageClient.cpp

@@ -64,7 +64,7 @@ void LanguageClient::open_file(ByteString const& path, int fd)
 {
     if (!m_connection_wrapper.connection())
         return;
-    m_connection_wrapper.connection()->async_file_opened(path, fd);
+    m_connection_wrapper.connection()->async_file_opened(path, MUST(IPC::File::clone_fd(fd)));
 }
 
 void LanguageClient::set_file_content(ByteString const& path, ByteString const& content)

Userland/Libraries/LibGUI/Window.cpp

@@ -983,7 +983,7 @@ void Window::set_current_backing_store(WindowBackingStore& backing_store, bool f
         m_window_id,
         32,
         bitmap.pitch(),
-        bitmap.anonymous_buffer().fd(),
+        MUST(IPC::File::clone_fd(bitmap.anonymous_buffer().fd())),
         backing_store.serial(),
         bitmap.has_alpha_channel(),
         bitmap.size(),

Userland/Libraries/LibGfx/ShareableBitmap.cpp

@@ -30,7 +30,7 @@ ErrorOr<void> encode(Encoder& encoder, Gfx::ShareableBitmap const& shareable_bit
         return {};
 
     auto& bitmap = *shareable_bitmap.bitmap();
-    TRY(encoder.encode(IPC::File(bitmap.anonymous_buffer().fd())));
+    TRY(encoder.encode(TRY(IPC::File::clone_fd(bitmap.anonymous_buffer().fd()))));
     TRY(encoder.encode(bitmap.size()));
     TRY(encoder.encode(static_cast<u32>(bitmap.scale())));
     TRY(encoder.encode(static_cast<u32>(bitmap.format())));

Userland/Libraries/LibIPC/Decoder.cpp

@@ -91,7 +91,7 @@ template<>
 ErrorOr<File> decode(Decoder& decoder)
 {
     int fd = TRY(decoder.socket().receive_fd(O_CLOEXEC));
-    return File { fd, File::ConstructWithReceivedFileDescriptor };
+    return File::adopt_fd(fd);
 }
 
 template<>

Userland/Libraries/LibIPC/Encoder.cpp

@@ -105,10 +105,7 @@ ErrorOr<void> encode(Encoder& encoder, URL::URL const& value)
 template<>
 ErrorOr<void> encode(Encoder& encoder, File const& file)
 {
-    int fd = file.fd();
+    int fd = file.take_fd();
-
-    if (fd != -1)
-        fd = TRY(Core::System::dup(fd));
 
     TRY(encoder.append_file_descriptor(fd));
     return {};
@@ -127,7 +124,7 @@ ErrorOr<void> encode(Encoder& encoder, Core::AnonymousBuffer const& buffer)
 
     if (buffer.is_valid()) {
         TRY(encoder.encode_size(buffer.size()));
-        TRY(encoder.encode(IPC::File { buffer.fd() }));
+        TRY(encoder.encode(TRY(IPC::File::clone_fd(buffer.fd()))));
     }
 
     return {};

Userland/Libraries/LibIPC/Encoder.h

@@ -13,6 +13,7 @@
 #include <AK/Variant.h>
 #include <LibCore/SharedCircularQueue.h>
 #include <LibIPC/Concepts.h>
+#include <LibIPC/File.h>
 #include <LibIPC/Forward.h>
 #include <LibIPC/Message.h>
 #include <LibURL/Forward.h>
@@ -148,7 +149,8 @@ ErrorOr<void> encode(Encoder& encoder, T const& hashmap)
 template<Concepts::SharedSingleProducerCircularQueue T>
 ErrorOr<void> encode(Encoder& encoder, T const& queue)
 {
-    return encoder.encode(IPC::File { queue.fd() });
+    TRY(encoder.encode(TRY(IPC::File::clone_fd(queue.fd()))));
+    return {};
 }
 
 template<Concepts::Optional T>

Userland/Libraries/LibIPC/File.h

@@ -10,7 +10,7 @@
 #include <AK/Noncopyable.h>
 #include <AK/StdLibExtras.h>
 #include <LibCore/File.h>
-#include <unistd.h>
+#include <LibCore/System.h>
 
 namespace IPC {
 
@@ -18,36 +18,26 @@ class File {
     AK_MAKE_NONCOPYABLE(File);
 
 public:
-    // Must have a default constructor, because LibIPC
-    // default-constructs arguments prior to decoding them.
     File() = default;
 
-    // Intentionally not `explicit`.
+    static File adopt_file(NonnullOwnPtr<Core::File> file)
-    File(int fd)
-        : m_fd(fd)
     {
+        return File(file->leak_fd(Badge<File> {}));
     }
 
-    // Tagged constructor for fd's that should be closed on destruction unless take_fd() is called.
+    static File adopt_fd(int fd)
-    // Note that the tags are the same, this is intentional to allow expressive invocation.
-    enum Tag {
-        ConstructWithReceivedFileDescriptor = 1,
-        CloseAfterSending = 1,
-    };
-    File(int fd, Tag)
-        : m_fd(fd)
-        , m_close_on_destruction(true)
     {
+        return File(fd);
     }
 
-    explicit File(Core::File& file)
+    static ErrorOr<File> clone_fd(int fd)
-        : File(file.leak_fd(Badge<File> {}), CloseAfterSending)
     {
+        int new_fd = TRY(Core::System::dup(fd));
+        return File(new_fd);
     }
 
     File(File&& other)
         : m_fd(exchange(other.m_fd, -1))
-        , m_close_on_destruction(exchange(other.m_close_on_destruction, false))
     {
     }
 
@@ -55,15 +45,14 @@ public:
     {
         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)
+        if (m_fd != -1)
-            close(m_fd);
+            (void)Core::System::close(m_fd);
     }
 
     int fd() const { return m_fd; }
@@ -75,8 +64,12 @@ public:
     }
 
 private:
+    explicit File(int fd)
+        : m_fd(fd)
+    {
+    }
+
     mutable int m_fd { -1 };
-    bool m_close_on_destruction { false };
 };
 
 }

Userland/Libraries/LibWeb/HTML/MessagePort.cpp

@@ -80,12 +80,12 @@ WebIDL::ExceptionOr<void> MessagePort::transfer_steps(HTML::TransferDataHolder&
         // 2. Set dataHolder.[[RemotePort]] to remotePort.
         auto fd = MUST(m_socket->release_fd());
         m_socket = nullptr;
-        data_holder.fds.append(IPC::File(fd, IPC::File::CloseAfterSending));
+        data_holder.fds.append(IPC::File::adopt_fd(fd));
         data_holder.data.append(IPC_FILE_TAG);
 
         auto fd_passing_socket = MUST(m_fd_passing_socket->release_fd());
         m_fd_passing_socket = nullptr;
-        data_holder.fds.append(IPC::File(fd_passing_socket, IPC::File::CloseAfterSending));
+        data_holder.fds.append(IPC::File::adopt_fd(fd_passing_socket));
         data_holder.data.append(IPC_FILE_TAG);
     }
 

Userland/Libraries/LibWeb/HTML/SelectedFile.cpp

@@ -20,7 +20,7 @@ ErrorOr<SelectedFile> SelectedFile::from_file_path(ByteString const& file_path)
     auto name = LexicalPath::basename(file_path);
 
     auto file = TRY(Core::File::open(file_path, Core::File::OpenMode::Read));
-    return SelectedFile { move(name), IPC::File { *file } };
+    return SelectedFile { move(name), IPC::File::adopt_file(move(file)) };
 }
 
 SelectedFile::SelectedFile(ByteString name, ByteBuffer contents)

Userland/Libraries/LibWeb/Page/Page.h

@@ -315,7 +315,7 @@ public:
 
     virtual void page_did_change_audio_play_state(HTML::AudioPlayState) { }
 
-    virtual WebView::SocketPair request_worker_agent() { return { -1, -1 }; }
+    virtual WebView::SocketPair request_worker_agent() { return { IPC::File {}, IPC::File {} }; }
 
     virtual void inspector_did_load() { }
     virtual void inspector_did_select_dom_node([[maybe_unused]] i32 node_id, [[maybe_unused]] Optional<CSS::Selector::PseudoElement::Type> const& pseudo_element) { }

Userland/Libraries/LibWeb/Worker/WebWorkerClient.cpp

@@ -22,8 +22,8 @@ WebWorkerClient::WebWorkerClient(NonnullOwnPtr<Core::LocalSocket> socket)
 WebView::SocketPair WebWorkerClient::dup_sockets()
 {
     WebView::SocketPair pair;
-    pair.socket = IPC::File(MUST(Core::System::dup(socket().fd().value())), IPC::File::CloseAfterSending);
+    pair.socket = MUST(IPC::File::clone_fd(socket().fd().value()));
-    pair.fd_passing_socket = IPC::File(MUST(Core::System::dup(fd_passing_socket().fd().value())), IPC::File::CloseAfterSending);
+    pair.fd_passing_socket = MUST(IPC::File::clone_fd(fd_passing_socket().fd().value()));
     return pair;
 }
 

Userland/Libraries/LibWebView/OutOfProcessWebView.cpp

@@ -44,7 +44,7 @@ OutOfProcessWebView::OutOfProcessWebView()
         if (file.is_error())
             client().async_handle_file_return(m_client_state.page_index, file.error().code(), {}, request_id);
         else
-            client().async_handle_file_return(m_client_state.page_index, 0, IPC::File(file.value().stream()), request_id);
+            client().async_handle_file_return(m_client_state.page_index, 0, IPC::File::adopt_file(file.release_value().release_stream()), request_id);
     };
 
     on_scroll_by_delta = [this](auto x_delta, auto y_delta) {

Userland/Libraries/LibWebView/ViewImplementation.cpp

@@ -32,7 +32,7 @@ ViewImplementation::ViewImplementation()
         if (file.is_error())
             client().async_handle_file_return(page_id(), file.error().code(), {}, request_id);
         else
-            client().async_handle_file_return(page_id(), 0, IPC::File(*file.value()), request_id);
+            client().async_handle_file_return(page_id(), 0, IPC::File::adopt_file(file.release_value()), request_id);
     };
 }
 

Userland/Libraries/LibWebView/WebContentClient.cpp

@@ -665,7 +665,7 @@ Messages::WebContentClient::RequestWorkerAgentResponse WebContentClient::request
             return view->on_request_worker_agent();
     }
 
-    return WebView::SocketPair { -1, -1 };
+    return WebView::SocketPair { IPC::File {}, IPC::File {} };
 }
 
 Optional<ViewImplementation&> WebContentClient::view_for_page_id(u64 page_id, SourceLocation location)

Userland/Services/FileSystemAccessServer/ConnectionFromClient.cpp

@@ -81,7 +81,7 @@ void ConnectionFromClient::request_file_handler(i32 request_id, i32 window_serve
             dbgln("FileSystemAccessServer: Couldn't open {}, error {}", path, file.error());
             async_handle_prompt_end(request_id, file.error().code(), Optional<IPC::File> {}, path);
         } else {
-            async_handle_prompt_end(request_id, 0, IPC::File(*file.release_value()), path);
+            async_handle_prompt_end(request_id, 0, IPC::File::adopt_file(file.release_value()), path);
         }
     } else {
         async_handle_prompt_end(request_id, EPERM, Optional<IPC::File> {}, path);
@@ -138,7 +138,7 @@ void ConnectionFromClient::prompt_helper(i32 request_id, Optional<ByteString> co
 
             m_approved_files.set(user_picked_file.value(), new_permissions);
 
-            async_handle_prompt_end(request_id, 0, IPC::File(*file.release_value()), user_picked_file);
+            async_handle_prompt_end(request_id, 0, IPC::File::adopt_file(file.release_value()), user_picked_file);
         }
     } else {
         async_handle_prompt_end(request_id, ECANCELED, Optional<IPC::File> {}, Optional<ByteString> {});

Userland/Services/RequestServer/ConnectionFromClient.cpp

@@ -42,10 +42,12 @@ void ConnectionFromClient::die()
 
 Messages::RequestServer::ConnectNewClientResponse ConnectionFromClient::connect_new_client()
 {
+    Messages::RequestServer::ConnectNewClientResponse error_response = { IPC::File {}, IPC::File {} };
+
     int socket_fds[2] {};
     if (auto err = Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, socket_fds); err.is_error()) {
         dbgln("Failed to create client socketpair: {}", err.error());
-        return { -1, -1 };
+        return error_response;
     }
 
     auto client_socket_or_error = Core::LocalSocket::adopt_fd(socket_fds[0]);
@@ -53,7 +55,7 @@ Messages::RequestServer::ConnectNewClientResponse ConnectionFromClient::connect_
         close(socket_fds[0]);
         close(socket_fds[1]);
         dbgln("Failed to adopt client socket: {}", client_socket_or_error.error());
-        return { -1, -1 };
+        return error_response;
     }
     auto client_socket = client_socket_or_error.release_value();
     // Note: A ref is stored in the static s_connections map
@@ -63,7 +65,7 @@ Messages::RequestServer::ConnectNewClientResponse ConnectionFromClient::connect_
     if (auto err = Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, fd_passing_socket_fds); err.is_error()) {
         close(socket_fds[1]);
         dbgln("Failed to create fd-passing socketpair: {}", err.error());
-        return { -1, -1 };
+        return error_response;
     }
 
     auto fd_passing_socket_or_error = Core::LocalSocket::adopt_fd(fd_passing_socket_fds[0]);
@@ -73,12 +75,12 @@ Messages::RequestServer::ConnectNewClientResponse ConnectionFromClient::connect_
         close(fd_passing_socket_fds[0]);
         close(fd_passing_socket_fds[1]);
         dbgln("Failed to adopt fd-passing socket: {}", fd_passing_socket_or_error.error());
-        return { -1, -1 };
+        return error_response;
     }
     auto fd_passing_socket = fd_passing_socket_or_error.release_value();
     client->set_fd_passing_socket(move(fd_passing_socket));
 
-    return { IPC::File(socket_fds[1], IPC::File::CloseAfterSending), IPC::File(fd_passing_socket_fds[1], IPC::File::CloseAfterSending) };
+    return { IPC::File::adopt_fd(socket_fds[1]), IPC::File::adopt_fd(fd_passing_socket_fds[1]) };
 }
 
 Messages::RequestServer::IsSupportedProtocolResponse ConnectionFromClient::is_supported_protocol(ByteString const& protocol)
@@ -110,7 +112,7 @@ void ConnectionFromClient::start_request(i32 request_id, ByteString const& metho
     auto id = request->id();
     auto fd = request->request_fd();
     m_requests.set(id, move(request));
-    (void)post_message(Messages::RequestClient::RequestStarted(request_id, IPC::File(fd, IPC::File::CloseAfterSending)));
+    (void)post_message(Messages::RequestClient::RequestStarted(request_id, IPC::File::adopt_fd(fd)));
 }
 
 Messages::RequestServer::StopRequestResponse ConnectionFromClient::stop_request(i32 request_id)

Userland/Services/WebWorker/ConnectionFromClient.cpp

@@ -31,7 +31,7 @@ void ConnectionFromClient::request_file(Web::FileRequest request)
     if (file.is_error())
         handle_file_return(file.error().code(), {}, request_id);
     else
-        handle_file_return(0, IPC::File(*file.value()), request_id);
+        handle_file_return(0, IPC::File::adopt_file(file.release_value()), request_id);
 }
 
 ConnectionFromClient::ConnectionFromClient(NonnullOwnPtr<Core::LocalSocket> socket)