LibCore+Services: Make TCPServer propagate errors

Tests/LibCore/TestLibCoreStream.cpp

@@ -148,9 +148,11 @@ TEST_CASE(tcp_socket_read)
     //       Core::EventLoop through Core::Notifier.
     Core::EventLoop event_loop;
 
-    auto tcp_server = Core::TCPServer::construct();
+    auto maybe_tcp_server = Core::TCPServer::try_create();
-    EXPECT(tcp_server->listen({ 127, 0, 0, 1 }, 9090));
+    EXPECT(!maybe_tcp_server.is_error());
-    tcp_server->set_blocking(true);
+    auto tcp_server = maybe_tcp_server.release_value();
+    EXPECT(!tcp_server->listen({ 127, 0, 0, 1 }, 9090).is_error());
+    EXPECT(!tcp_server->set_blocking(true).is_error());
 
     auto maybe_client_socket = Core::Stream::TCPSocket::connect({ { 127, 0, 0, 1 }, 9090 });
     EXPECT(!maybe_client_socket.is_error());
@@ -181,9 +183,11 @@ TEST_CASE(tcp_socket_write)
 {
     Core::EventLoop event_loop;
 
-    auto tcp_server = Core::TCPServer::construct();
+    auto maybe_tcp_server = Core::TCPServer::try_create();
-    EXPECT(tcp_server->listen({ 127, 0, 0, 1 }, 9090));
+    EXPECT(!maybe_tcp_server.is_error());
-    tcp_server->set_blocking(true);
+    auto tcp_server = maybe_tcp_server.release_value();
+    EXPECT(!tcp_server->listen({ 127, 0, 0, 1 }, 9090).is_error());
+    EXPECT(!tcp_server->set_blocking(true).is_error());
 
     auto maybe_client_socket = Core::Stream::TCPSocket::connect({ { 127, 0, 0, 1 }, 9090 });
     EXPECT(!maybe_client_socket.is_error());
@@ -210,9 +214,11 @@ TEST_CASE(tcp_socket_eof)
 {
     Core::EventLoop event_loop;
 
-    auto tcp_server = Core::TCPServer::construct();
+    auto maybe_tcp_server = Core::TCPServer::try_create();
-    EXPECT(tcp_server->listen({ 127, 0, 0, 1 }, 9090));
+    EXPECT(!maybe_tcp_server.is_error());
-    tcp_server->set_blocking(true);
+    auto tcp_server = maybe_tcp_server.release_value();
+    EXPECT(!tcp_server->listen({ 127, 0, 0, 1 }, 9090).is_error());
+    EXPECT(!tcp_server->set_blocking(true).is_error());
 
     auto maybe_client_socket = Core::Stream::TCPSocket::connect({ { 127, 0, 0, 1 }, 9090 });
     EXPECT(!maybe_client_socket.is_error());
@@ -404,9 +410,11 @@ TEST_CASE(buffered_tcp_socket_read)
 {
     Core::EventLoop event_loop;
 
-    auto tcp_server = Core::TCPServer::construct();
+    auto maybe_tcp_server = Core::TCPServer::try_create();
-    EXPECT(tcp_server->listen({ 127, 0, 0, 1 }, 9090));
+    EXPECT(!maybe_tcp_server.is_error());
-    tcp_server->set_blocking(true);
+    auto tcp_server = maybe_tcp_server.release_value();
+    EXPECT(!tcp_server->listen({ 127, 0, 0, 1 }, 9090).is_error());
+    EXPECT(!tcp_server->set_blocking(true).is_error());
 
     auto maybe_client_socket = Core::Stream::TCPSocket::connect({ { 127, 0, 0, 1 }, 9090 });
     EXPECT(!maybe_client_socket.is_error());

Userland/Libraries/LibCore/TCPServer.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -7,53 +8,47 @@
 #include <AK/IPv4Address.h>
 #include <AK/Types.h>
 #include <LibCore/Notifier.h>
+#include <LibCore/System.h>
 #include <LibCore/TCPServer.h>
 #include <LibCore/TCPSocket.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <sys/socket.h>
-#include <unistd.h>
 
-#ifndef SOCK_NONBLOCK
-#    include <sys/ioctl.h>
-#endif
 namespace Core {
 
-TCPServer::TCPServer(Object* parent)
+ErrorOr<NonnullRefPtr<TCPServer>> TCPServer::try_create(Object* parent)
-    : Object(parent)
 {
 #ifdef SOCK_NONBLOCK
-    m_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
+    int fd = TRY(Core::System::socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
 #else
-    m_fd = socket(AF_INET, SOCK_STREAM, 0);
+    int fd = TRY(Core::System::socket(AF_INET, SOCK_STREAM, 0));
     int option = 1;
-    ioctl(m_fd, FIONBIO, &option);
+    TRY(Core::System::ioctl(fd, FIONBIO, &option));
-    fcntl(m_fd, F_SETFD, FD_CLOEXEC);
+    TRY(Core::System::fcntl(fd, F_SETFD, FD_CLOEXEC));
 #endif
+
+    return adopt_nonnull_ref_or_enomem(new (nothrow) TCPServer(fd, parent));
+}
+
+TCPServer::TCPServer(int fd, Object* parent)
+    : Object(parent)
+    , m_fd(fd)
+{
     VERIFY(m_fd >= 0);
 }
 
 TCPServer::~TCPServer()
 {
-    ::close(m_fd);
+    MUST(Core::System::close(m_fd));
 }
 
-bool TCPServer::listen(const IPv4Address& address, u16 port)
+ErrorOr<void> TCPServer::listen(IPv4Address const& address, u16 port)
 {
     if (m_listening)
-        return false;
+        return Error::from_errno(EADDRINUSE);
 
     auto socket_address = SocketAddress(address, port);
     auto in = socket_address.to_sockaddr_in();
-    if (::bind(m_fd, (const sockaddr*)&in, sizeof(in)) < 0) {
+    TRY(Core::System::bind(m_fd, (sockaddr const*)&in, sizeof(in)));
-        perror("TCPServer::listen: bind");
+    TRY(Core::System::listen(m_fd, 5));
-        return false;
-    }
-
-    if (::listen(m_fd, 5) < 0) {
-        perror("TCPServer::listen: listen");
-        return false;
-    }
     m_listening = true;
 
     m_notifier = Notifier::construct(m_fd, Notifier::Event::Read, this);
@@ -61,18 +56,17 @@ bool TCPServer::listen(const IPv4Address& address, u16 port)
         if (on_ready_to_accept)
             on_ready_to_accept();
     };
-    return true;
+    return {};
 }
 
-void TCPServer::set_blocking(bool blocking)
+ErrorOr<void> TCPServer::set_blocking(bool blocking)
 {
-    int flags = fcntl(m_fd, F_GETFL, 0);
+    int flags = TRY(Core::System::fcntl(m_fd, F_GETFL, 0));
-    VERIFY(flags >= 0);
     if (blocking)
-        flags = fcntl(m_fd, F_SETFL, flags & ~O_NONBLOCK);
+        TRY(Core::System::fcntl(m_fd, F_SETFL, flags & ~O_NONBLOCK));
     else
-        flags = fcntl(m_fd, F_SETFL, flags | O_NONBLOCK);
+        TRY(Core::System::fcntl(m_fd, F_SETFL, flags | O_NONBLOCK));
-    VERIFY(flags == 0);
+    return {};
 }
 
 ErrorOr<Stream::TCPSocket> TCPServer::accept()
@@ -81,15 +75,11 @@ ErrorOr<Stream::TCPSocket> TCPServer::accept()
     sockaddr_in in;
     socklen_t in_size = sizeof(in);
 #ifndef AK_OS_MACOS
-    int accepted_fd = ::accept4(m_fd, (sockaddr*)&in, &in_size, SOCK_NONBLOCK | SOCK_CLOEXEC);
+    int accepted_fd = TRY(Core::System::accept4(m_fd, (sockaddr*)&in, &in_size, SOCK_NONBLOCK | SOCK_CLOEXEC));
 #else
-    int accepted_fd = ::accept(m_fd, (sockaddr*)&in, &in_size);
+    int accepted_fd = TRY(Core::System::accept(m_fd, (sockaddr*)&in, &in_size));
 #endif
 
-    if (accepted_fd < 0) {
-        return Error::from_errno(errno);
-    }
-
     auto socket = TRY(Stream::TCPSocket::adopt_fd(accepted_fd));
 
 #ifdef AK_OS_MACOS

Userland/Libraries/LibCore/TCPServer.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -14,13 +15,14 @@
 namespace Core {
 
 class TCPServer : public Object {
-    C_OBJECT(TCPServer)
+    C_OBJECT_ABSTRACT(TCPServer)
 public:
+    static ErrorOr<NonnullRefPtr<TCPServer>> try_create(Object* parent = nullptr);
     virtual ~TCPServer() override;
 
     bool is_listening() const { return m_listening; }
-    bool listen(const IPv4Address& address, u16 port);
+    ErrorOr<void> listen(IPv4Address const& address, u16 port);
-    void set_blocking(bool blocking);
+    ErrorOr<void> set_blocking(bool blocking);
 
     ErrorOr<Stream::TCPSocket> accept();
 
@@ -30,7 +32,7 @@ public:
     Function<void()> on_ready_to_accept;
 
 private:
-    explicit TCPServer(Object* parent = nullptr);
+    explicit TCPServer(int fd, Object* parent = nullptr);
 
     int m_fd { -1 };
     bool m_listening { false };

Userland/Services/EchoServer/main.cpp

@@ -31,12 +31,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     Core::EventLoop event_loop;
 
-    auto server = Core::TCPServer::construct();
+    auto server = TRY(Core::TCPServer::try_create());
-
+    TRY(server->listen({}, port));
-    if (!server->listen({}, port)) {
-        warnln("Listening on 0.0.0.0:{} failed", port);
-        exit(1);
-    }
 
     HashMap<int, NonnullRefPtr<Client>> clients;
     int next_id = 0;

Userland/Services/TelnetServer/main.cpp

@@ -97,12 +97,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     }
 
     Core::EventLoop event_loop;
-    auto server = Core::TCPServer::construct();
+    auto server = TRY(Core::TCPServer::try_create());
-
+    TRY(server->listen({}, port));
-    if (!server->listen({}, port)) {
-        warnln("Listening on 0.0.0.0:{} failed", port);
-        exit(1);
-    }
 
     HashMap<int, NonnullRefPtr<Client>> clients;
     int next_id = 0;

Userland/Services/WebServer/main.cpp

@@ -89,10 +89,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         client->start();
     };
 
-    if (!server->listen(ipv4_address.value(), port)) {
+    TRY(server->listen(ipv4_address.value(), port));
-        warnln("Failed to listen on {}:{}", ipv4_address.value(), port);
-        return 1;
-    }
 
     outln("Listening on {}:{}", ipv4_address.value(), port);