Bläddra i källkod

LibCore+Userland+Tests: Convert Stream APIs to construct on heap

As per previous discussion, it was decided that the Stream classes
should be constructed on the heap.

While I don't personally agree with this change, it does have the
benefit of avoiding Function object reconstructions due to the lambda
passed to Notifier pointing to a stale object reference. This also has
the benefit of not having to "box" objects for virtual usage, as the
objects come pre-boxed.

However, it means that we now hit the heap everytime we construct a
TCPSocket for instance, which might not be desirable.
sin-ack 3 år sedan
förälder
incheckning
dbd25916a3

+ 63 - 63
Tests/LibCore/TestLibCoreStream.cpp

@@ -31,12 +31,12 @@ TEST_CASE(file_open)
 
     // Testing out some basic file properties.
     auto file = maybe_file.release_value();
-    EXPECT(file.is_open());
-    EXPECT(!file.is_readable());
-    EXPECT(file.is_writable());
-    EXPECT(!file.is_eof());
+    EXPECT(file->is_open());
+    EXPECT(!file->is_readable());
+    EXPECT(file->is_writable());
+    EXPECT(!file->is_eof());
 
-    auto maybe_size = file.size();
+    auto maybe_size = file->size();
     EXPECT(!maybe_size.is_error());
     EXPECT_EQ(maybe_size.value(), 0);
 }
@@ -48,7 +48,7 @@ TEST_CASE(file_write_bytes)
 
     constexpr auto some_words = "These are some words"sv;
     ReadonlyBytes buffer { some_words.characters_without_null_termination(), some_words.length() };
-    auto result = file.write(buffer);
+    auto result = file->write(buffer);
     EXPECT(!result.is_error());
 }
 
@@ -64,7 +64,7 @@ TEST_CASE(file_read_bytes)
     EXPECT(maybe_buffer.has_value());
     auto buffer = maybe_buffer.release_value();
 
-    auto result = file.read(buffer);
+    auto result = file->read(buffer);
     EXPECT(!result.is_error());
     EXPECT_EQ(result.value(), 131ul);
 
@@ -82,7 +82,7 @@ TEST_CASE(file_seeking_around)
     EXPECT(!maybe_file.is_error());
     auto file = maybe_file.release_value();
 
-    EXPECT_EQ(file.size().release_value(), 8702);
+    EXPECT_EQ(file->size().release_value(), 8702);
 
     auto maybe_buffer = ByteBuffer::create_uninitialized(16);
     EXPECT(maybe_buffer.has_value());
@@ -90,19 +90,19 @@ TEST_CASE(file_seeking_around)
 
     StringView buffer_contents { buffer.bytes() };
 
-    EXPECT(!file.seek(500, Core::Stream::SeekMode::SetPosition).is_error());
-    EXPECT_EQ(file.tell().release_value(), 500);
-    EXPECT(file.read_or_error(buffer));
+    EXPECT(!file->seek(500, Core::Stream::SeekMode::SetPosition).is_error());
+    EXPECT_EQ(file->tell().release_value(), 500);
+    EXPECT(file->read_or_error(buffer));
     EXPECT_EQ(buffer_contents, expected_seek_contents1);
 
-    EXPECT(!file.seek(234, Core::Stream::SeekMode::FromCurrentPosition).is_error());
-    EXPECT_EQ(file.tell().release_value(), 750);
-    EXPECT(file.read_or_error(buffer));
+    EXPECT(!file->seek(234, Core::Stream::SeekMode::FromCurrentPosition).is_error());
+    EXPECT_EQ(file->tell().release_value(), 750);
+    EXPECT(file->read_or_error(buffer));
     EXPECT_EQ(buffer_contents, expected_seek_contents2);
 
-    EXPECT(!file.seek(-105, Core::Stream::SeekMode::FromEndPosition).is_error());
-    EXPECT_EQ(file.tell().release_value(), 8597);
-    EXPECT(file.read_or_error(buffer));
+    EXPECT(!file->seek(-105, Core::Stream::SeekMode::FromEndPosition).is_error());
+    EXPECT_EQ(file->tell().release_value(), 8597);
+    EXPECT(file->read_or_error(buffer));
     EXPECT_EQ(buffer_contents, expected_seek_contents3);
 }
 
@@ -115,7 +115,7 @@ TEST_CASE(file_adopt_fd)
     EXPECT(!maybe_file.is_error());
     auto file = maybe_file.release_value();
 
-    EXPECT_EQ(file.size().release_value(), 8702);
+    EXPECT_EQ(file->size().release_value(), 8702);
 
     auto maybe_buffer = ByteBuffer::create_uninitialized(16);
     EXPECT(maybe_buffer.has_value());
@@ -123,9 +123,9 @@ TEST_CASE(file_adopt_fd)
 
     StringView buffer_contents { buffer.bytes() };
 
-    EXPECT(!file.seek(500, Core::Stream::SeekMode::SetPosition).is_error());
-    EXPECT_EQ(file.tell().release_value(), 500);
-    EXPECT(file.read_or_error(buffer));
+    EXPECT(!file->seek(500, Core::Stream::SeekMode::SetPosition).is_error());
+    EXPECT_EQ(file->tell().release_value(), 500);
+    EXPECT(file->read_or_error(buffer));
     EXPECT_EQ(buffer_contents, expected_seek_contents1);
 
     // A single seek & read test should be fine for now.
@@ -158,20 +158,20 @@ TEST_CASE(tcp_socket_read)
     EXPECT(!maybe_client_socket.is_error());
     auto client_socket = maybe_client_socket.release_value();
 
-    EXPECT(client_socket.is_open());
+    EXPECT(client_socket->is_open());
 
     auto maybe_server_socket = tcp_server->accept();
     EXPECT(!maybe_server_socket.is_error());
     auto server_socket = maybe_server_socket.release_value();
-    EXPECT(!server_socket.write({ sent_data.characters_without_null_termination(), sent_data.length() }).is_error());
-    server_socket.close();
+    EXPECT(!server_socket->write({ sent_data.characters_without_null_termination(), sent_data.length() }).is_error());
+    server_socket->close();
 
-    EXPECT(client_socket.can_read_without_blocking(100).release_value());
-    EXPECT_EQ(client_socket.pending_bytes().release_value(), sent_data.length());
+    EXPECT(client_socket->can_read_without_blocking(100).release_value());
+    EXPECT_EQ(client_socket->pending_bytes().release_value(), sent_data.length());
 
     auto maybe_receive_buffer = ByteBuffer::create_uninitialized(64);
     auto receive_buffer = maybe_receive_buffer.release_value();
-    auto maybe_nread = client_socket.read(receive_buffer);
+    auto maybe_nread = client_socket->read(receive_buffer);
     EXPECT(!maybe_nread.is_error());
     auto nread = maybe_nread.release_value();
 
@@ -196,13 +196,13 @@ TEST_CASE(tcp_socket_write)
     auto maybe_server_socket = tcp_server->accept();
     EXPECT(!maybe_server_socket.is_error());
     auto server_socket = maybe_server_socket.release_value();
-    EXPECT(!server_socket.set_blocking(true).is_error());
+    EXPECT(!server_socket->set_blocking(true).is_error());
 
-    EXPECT(client_socket.write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
-    client_socket.close();
+    EXPECT(client_socket->write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
+    client_socket->close();
 
     auto receive_buffer = ByteBuffer::create_uninitialized(64).release_value();
-    auto maybe_nread = server_socket.read(receive_buffer);
+    auto maybe_nread = server_socket->read(receive_buffer);
     EXPECT(!maybe_nread.is_error());
     auto nread = maybe_nread.release_value();
 
@@ -224,21 +224,21 @@ TEST_CASE(tcp_socket_eof)
     EXPECT(!maybe_client_socket.is_error());
     auto client_socket = maybe_client_socket.release_value();
 
-    EXPECT(client_socket.is_open());
+    EXPECT(client_socket->is_open());
 
     auto server_socket = tcp_server->accept().release_value();
-    server_socket.close();
+    server_socket->close();
 
     // NOTE: This may seem unintuitive, but poll will mark a fd which has
     //       reached EOF (i.e. in the case of the other side disconnecting) as
     //       POLLIN.
-    EXPECT(client_socket.can_read_without_blocking(100).release_value());
-    EXPECT_EQ(client_socket.pending_bytes().release_value(), 0ul);
+    EXPECT(client_socket->can_read_without_blocking(100).release_value());
+    EXPECT_EQ(client_socket->pending_bytes().release_value(), 0ul);
 
     auto maybe_receive_buffer = ByteBuffer::create_uninitialized(1);
     auto receive_buffer = maybe_receive_buffer.release_value();
-    EXPECT_EQ(client_socket.read(receive_buffer).release_value(), 0ul);
-    EXPECT(client_socket.is_eof());
+    EXPECT_EQ(client_socket->read(receive_buffer).release_value(), 0ul);
+    EXPECT(client_socket->is_eof());
 }
 
 // UDPSocket tests
@@ -258,8 +258,8 @@ TEST_CASE(udp_socket_read_write)
     EXPECT(!maybe_client_socket.is_error());
     auto client_socket = maybe_client_socket.release_value();
 
-    EXPECT(client_socket.is_open());
-    EXPECT(client_socket.write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
+    EXPECT(client_socket->is_open());
+    EXPECT(client_socket->write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
 
     // FIXME: UDPServer::receive sadly doesn't give us a way to block on it,
     // currently.
@@ -274,16 +274,16 @@ TEST_CASE(udp_socket_read_write)
 
     EXPECT(!udp_server->send({ udp_reply_data.characters_without_null_termination(), udp_reply_data.length() }, client_address).is_error());
 
-    EXPECT(client_socket.can_read_without_blocking(100).release_value());
-    EXPECT_EQ(client_socket.pending_bytes().release_value(), udp_reply_data.length());
+    EXPECT(client_socket->can_read_without_blocking(100).release_value());
+    EXPECT_EQ(client_socket->pending_bytes().release_value(), udp_reply_data.length());
 
     // Testing that supplying a smaller buffer than required causes a failure.
     auto small_buffer = ByteBuffer::create_uninitialized(8).release_value();
-    EXPECT_EQ(client_socket.read(small_buffer).error().code(), EMSGSIZE);
+    EXPECT_EQ(client_socket->read(small_buffer).error().code(), EMSGSIZE);
 
     auto maybe_client_receive_buffer = ByteBuffer::create_uninitialized(64);
     auto client_receive_buffer = maybe_client_receive_buffer.release_value();
-    auto maybe_nread = client_socket.read(client_receive_buffer);
+    auto maybe_nread = client_socket->read(client_receive_buffer);
     EXPECT(!maybe_nread.is_error());
     auto nread = maybe_nread.release_value();
 
@@ -317,14 +317,14 @@ TEST_CASE(local_socket_read)
             EXPECT(!maybe_client_socket.is_error());
             auto client_socket = maybe_client_socket.release_value();
 
-            EXPECT(client_socket.is_open());
+            EXPECT(client_socket->is_open());
 
-            EXPECT(client_socket.can_read_without_blocking(100).release_value());
-            EXPECT_EQ(client_socket.pending_bytes().release_value(), sent_data.length());
+            EXPECT(client_socket->can_read_without_blocking(100).release_value());
+            EXPECT_EQ(client_socket->pending_bytes().release_value(), sent_data.length());
 
             auto maybe_receive_buffer = ByteBuffer::create_uninitialized(64);
             auto receive_buffer = maybe_receive_buffer.release_value();
-            auto maybe_nread = client_socket.read(receive_buffer);
+            auto maybe_nread = client_socket->read(receive_buffer);
             EXPECT(!maybe_nread.is_error());
             auto nread = maybe_nread.release_value();
 
@@ -367,8 +367,8 @@ TEST_CASE(local_socket_write)
             EXPECT(!maybe_client_socket.is_error());
             auto client_socket = maybe_client_socket.release_value();
 
-            EXPECT(client_socket.write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
-            client_socket.close();
+            EXPECT(client_socket->write_or_error({ sent_data.characters_without_null_termination(), sent_data.length() }));
+            client_socket->close();
 
             return 0;
         },
@@ -389,15 +389,15 @@ TEST_CASE(buffered_long_file_read)
     auto file = maybe_buffered_file.release_value();
 
     auto buffer = ByteBuffer::create_uninitialized(4096).release_value();
-    EXPECT(!file.seek(255, Core::Stream::SeekMode::SetPosition).is_error());
-    EXPECT(file.can_read_line().release_value());
-    auto maybe_nread = file.read_line(buffer);
+    EXPECT(!file->seek(255, Core::Stream::SeekMode::SetPosition).is_error());
+    EXPECT(file->can_read_line().release_value());
+    auto maybe_nread = file->read_line(buffer);
     EXPECT(!maybe_nread.is_error());
     EXPECT_EQ(maybe_nread.value(), 4095ul); // 4095 bytes on the third line
 
     // Testing that buffering with seeking works properly
-    EXPECT(!file.seek(365, Core::Stream::SeekMode::SetPosition).is_error());
-    auto maybe_after_seek_nread = file.read_line(buffer);
+    EXPECT(!file->seek(365, Core::Stream::SeekMode::SetPosition).is_error());
+    auto maybe_after_seek_nread = file->read_line(buffer);
     EXPECT(!maybe_after_seek_nread.is_error());
     EXPECT_EQ(maybe_after_seek_nread.value(), 3985ul); // 4095 - 110
 }
@@ -420,14 +420,14 @@ TEST_CASE(buffered_small_file_read)
     // Testing that we don't read out of bounds when the entire file fits into the buffer
     auto buffer = ByteBuffer::create_uninitialized(4096).release_value();
     for (auto const& line : expected_lines) {
-        VERIFY(file.can_read_line().release_value());
-        auto maybe_nread = file.read_line(buffer);
+        VERIFY(file->can_read_line().release_value());
+        auto maybe_nread = file->read_line(buffer);
         EXPECT(!maybe_nread.is_error());
         EXPECT_EQ(maybe_nread.value(), line.length());
         EXPECT_EQ(StringView(buffer.span().trim(maybe_nread.value())), line);
     }
-    EXPECT(!file.can_read_line().is_error());
-    EXPECT(!file.can_read_line().value());
+    EXPECT(!file->can_read_line().is_error());
+    EXPECT(!file->can_read_line().value());
 }
 
 constexpr auto buffered_sent_data = "Well hello friends!\n:^)\nThis shouldn't be present. :^("sv;
@@ -450,23 +450,23 @@ TEST_CASE(buffered_tcp_socket_read)
     EXPECT(!maybe_buffered_socket.is_error());
     auto client_socket = maybe_buffered_socket.release_value();
 
-    EXPECT(client_socket.is_open());
+    EXPECT(client_socket->is_open());
 
     auto maybe_server_socket = tcp_server->accept();
     EXPECT(!maybe_server_socket.is_error());
     auto server_socket = maybe_server_socket.release_value();
-    EXPECT(!server_socket.write({ buffered_sent_data.characters_without_null_termination(), sent_data.length() }).is_error());
+    EXPECT(!server_socket->write({ buffered_sent_data.characters_without_null_termination(), sent_data.length() }).is_error());
 
-    EXPECT(client_socket.can_read_without_blocking(100).release_value());
+    EXPECT(client_socket->can_read_without_blocking(100).release_value());
 
     auto receive_buffer = ByteBuffer::create_uninitialized(64).release_value();
 
-    auto maybe_first_nread = client_socket.read_line(receive_buffer);
+    auto maybe_first_nread = client_socket->read_line(receive_buffer);
     EXPECT(!maybe_first_nread.is_error());
     StringView first_received_line { receive_buffer.data(), maybe_first_nread.value() };
     EXPECT_EQ(first_received_line, first_line);
 
-    auto maybe_second_nread = client_socket.read_line(receive_buffer);
+    auto maybe_second_nread = client_socket->read_line(receive_buffer);
     EXPECT(!maybe_second_nread.is_error());
     StringView second_received_line { receive_buffer.data(), maybe_second_nread.value() };
     EXPECT_EQ(second_received_line, second_line);

+ 2 - 2
Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp

@@ -180,8 +180,8 @@ ErrorOr<void> KeyboardMapperWidget::save_to_file(StringView filename)
     // Write to file.
     String file_content = map_json.to_string();
     auto file = TRY(Core::Stream::File::open(filename, Core::Stream::OpenMode::Write));
-    TRY(file.write(file_content.bytes()));
-    file.close();
+    TRY(file->write(file_content.bytes()));
+    file->close();
 
     m_modified = false;
     m_filename = filename;

+ 24 - 24
Userland/Libraries/LibCore/Stream.cpp

@@ -90,14 +90,14 @@ ErrorOr<off_t> SeekableStream::size()
     return seek_result.value();
 }
 
-ErrorOr<File> File::open(StringView const& filename, OpenMode mode, mode_t permissions)
+ErrorOr<NonnullOwnPtr<File>> File::open(StringView const& filename, OpenMode mode, mode_t permissions)
 {
-    File file { mode };
-    TRY(file.open_path(filename, permissions));
+    auto file = TRY(adopt_nonnull_own_or_enomem(new (nothrow) File(mode)));
+    TRY(file->open_path(filename, permissions));
     return file;
 }
 
-ErrorOr<File> File::adopt_fd(int fd, OpenMode mode)
+ErrorOr<NonnullOwnPtr<File>> File::adopt_fd(int fd, OpenMode mode)
 {
     if (fd < 0) {
         return Error::from_errno(EBADF);
@@ -108,8 +108,8 @@ ErrorOr<File> File::adopt_fd(int fd, OpenMode mode)
         return Error::from_errno(EINVAL);
     }
 
-    File file { mode };
-    file.m_fd = fd;
+    auto file = TRY(adopt_nonnull_own_or_enomem(new (nothrow) File(mode)));
+    file->m_fd = fd;
     return file;
 }
 
@@ -445,18 +445,18 @@ void PosixSocketHelper::setup_notifier()
         m_notifier = Core::Notifier::construct(m_fd, Core::Notifier::Read);
 }
 
-ErrorOr<TCPSocket> TCPSocket::connect(String const& host, u16 port)
+ErrorOr<NonnullOwnPtr<TCPSocket>> TCPSocket::connect(String const& host, u16 port)
 {
     auto ip_address = TRY(resolve_host(host, SocketType::Stream));
     return connect(SocketAddress { ip_address, port });
 }
 
-ErrorOr<TCPSocket> TCPSocket::connect(SocketAddress const& address)
+ErrorOr<NonnullOwnPtr<TCPSocket>> TCPSocket::connect(SocketAddress const& address)
 {
-    TCPSocket socket;
+    auto socket = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TCPSocket()));
 
     auto fd = TRY(create_fd(SocketDomain::Inet, SocketType::Stream));
-    socket.m_helper.set_fd(fd);
+    socket->m_helper.set_fd(fd);
 
     auto result = connect_inet(fd, address);
     if (result.is_error()) {
@@ -464,19 +464,19 @@ ErrorOr<TCPSocket> TCPSocket::connect(SocketAddress const& address)
         return result.release_error();
     }
 
-    socket.setup_notifier();
+    socket->setup_notifier();
     return socket;
 }
 
-ErrorOr<TCPSocket> TCPSocket::adopt_fd(int fd)
+ErrorOr<NonnullOwnPtr<TCPSocket>> TCPSocket::adopt_fd(int fd)
 {
     if (fd < 0) {
         return Error::from_errno(EBADF);
     }
 
-    TCPSocket socket;
-    socket.m_helper.set_fd(fd);
-    socket.setup_notifier();
+    auto socket = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TCPSocket()));
+    socket->m_helper.set_fd(fd);
+    socket->setup_notifier();
     return socket;
 }
 
@@ -495,18 +495,18 @@ ErrorOr<size_t> PosixSocketHelper::pending_bytes() const
     return static_cast<size_t>(value);
 }
 
-ErrorOr<UDPSocket> UDPSocket::connect(String const& host, u16 port)
+ErrorOr<NonnullOwnPtr<UDPSocket>> UDPSocket::connect(String const& host, u16 port)
 {
     auto ip_address = TRY(resolve_host(host, SocketType::Datagram));
     return connect(SocketAddress { ip_address, port });
 }
 
-ErrorOr<UDPSocket> UDPSocket::connect(SocketAddress const& address)
+ErrorOr<NonnullOwnPtr<UDPSocket>> UDPSocket::connect(SocketAddress const& address)
 {
-    UDPSocket socket;
+    auto socket = TRY(adopt_nonnull_own_or_enomem(new (nothrow) UDPSocket()));
 
     auto fd = TRY(create_fd(SocketDomain::Inet, SocketType::Datagram));
-    socket.m_helper.set_fd(fd);
+    socket->m_helper.set_fd(fd);
 
     auto result = connect_inet(fd, address);
     if (result.is_error()) {
@@ -514,16 +514,16 @@ ErrorOr<UDPSocket> UDPSocket::connect(SocketAddress const& address)
         return result.release_error();
     }
 
-    socket.setup_notifier();
+    socket->setup_notifier();
     return socket;
 }
 
-ErrorOr<LocalSocket> LocalSocket::connect(String const& path)
+ErrorOr<NonnullOwnPtr<LocalSocket>> LocalSocket::connect(String const& path)
 {
-    LocalSocket socket;
+    auto socket = TRY(adopt_nonnull_own_or_enomem(new (nothrow) LocalSocket()));
 
     auto fd = TRY(create_fd(SocketDomain::Local, SocketType::Stream));
-    socket.m_helper.set_fd(fd);
+    socket->m_helper.set_fd(fd);
 
     auto result = connect_local(fd, path);
     if (result.is_error()) {
@@ -531,7 +531,7 @@ ErrorOr<LocalSocket> LocalSocket::connect(String const& path)
         return result.release_error();
     }
 
-    socket.setup_notifier();
+    socket->setup_notifier();
     return socket;
 }
 

+ 27 - 27
Userland/Libraries/LibCore/Stream.h

@@ -166,8 +166,8 @@ class File final : public SeekableStream {
     AK_MAKE_NONCOPYABLE(File);
 
 public:
-    static ErrorOr<File> open(StringView const& filename, OpenMode, mode_t = 0644);
-    static ErrorOr<File> adopt_fd(int fd, OpenMode);
+    static ErrorOr<NonnullOwnPtr<File>> open(StringView const& filename, OpenMode, mode_t = 0644);
+    static ErrorOr<NonnullOwnPtr<File>> adopt_fd(int fd, OpenMode);
 
     File(File&& other) { operator=(move(other)); }
 
@@ -253,9 +253,9 @@ private:
 
 class TCPSocket final : public Socket {
 public:
-    static ErrorOr<TCPSocket> connect(String const& host, u16 port);
-    static ErrorOr<TCPSocket> connect(SocketAddress const& address);
-    static ErrorOr<TCPSocket> adopt_fd(int fd);
+    static ErrorOr<NonnullOwnPtr<TCPSocket>> connect(String const& host, u16 port);
+    static ErrorOr<NonnullOwnPtr<TCPSocket>> connect(SocketAddress const& address);
+    static ErrorOr<NonnullOwnPtr<TCPSocket>> adopt_fd(int fd);
 
     TCPSocket(TCPSocket&& other)
         : Socket(static_cast<Socket&&>(other))
@@ -310,8 +310,8 @@ private:
 
 class UDPSocket final : public Socket {
 public:
-    static ErrorOr<UDPSocket> connect(String const& host, u16 port);
-    static ErrorOr<UDPSocket> connect(SocketAddress const& address);
+    static ErrorOr<NonnullOwnPtr<UDPSocket>> connect(String const& host, u16 port);
+    static ErrorOr<NonnullOwnPtr<UDPSocket>> connect(SocketAddress const& address);
 
     UDPSocket(UDPSocket&& other)
         : Socket(static_cast<Socket&&>(other))
@@ -378,7 +378,7 @@ private:
 
 class LocalSocket final : public Socket {
 public:
-    static ErrorOr<LocalSocket> connect(String const& path);
+    static ErrorOr<NonnullOwnPtr<LocalSocket>> connect(String const& path);
 
     LocalSocket(LocalSocket&& other)
         : Socket(static_cast<Socket&&>(other))
@@ -444,7 +444,7 @@ class BufferedHelper {
 
 public:
     template<StreamLike U>
-    BufferedHelper(Badge<U>, T stream, ByteBuffer buffer)
+    BufferedHelper(Badge<U>, NonnullOwnPtr<T> stream, ByteBuffer buffer)
         : m_stream(move(stream))
         , m_buffer(move(buffer))
     {
@@ -466,22 +466,22 @@ public:
     }
 
     template<template<typename> typename BufferedType>
-    static ErrorOr<BufferedType<T>> create_buffered(T&& stream, size_t buffer_size)
+    static ErrorOr<NonnullOwnPtr<BufferedType<T>>> create_buffered(NonnullOwnPtr<T> stream, size_t buffer_size)
     {
         if (!buffer_size)
             return EINVAL;
-        if (!stream.is_open())
+        if (!stream->is_open())
             return ENOTCONN;
 
         auto maybe_buffer = ByteBuffer::create_uninitialized(buffer_size);
         if (!maybe_buffer.has_value())
             return ENOMEM;
 
-        return BufferedType<T> { move(stream), maybe_buffer.release_value() };
+        return adopt_nonnull_own_or_enomem(new BufferedType<T>(move(stream), maybe_buffer.release_value()));
     }
 
-    T& stream() { return m_stream; }
-    T const& stream() const { return m_stream; }
+    T& stream() { return *m_stream; }
+    T const& stream() const { return *m_stream; }
 
     ErrorOr<size_t> read(Bytes buffer)
     {
@@ -515,7 +515,7 @@ public:
 
         // Otherwise, let's try an extra read just in case there's something
         // in our receive buffer.
-        auto stream_nread = TRY(m_stream.read(buffer.slice(buffer_nread)));
+        auto stream_nread = TRY(stream().read(buffer.slice(buffer_nread)));
         return buffer_nread + stream_nread;
     }
 
@@ -661,12 +661,12 @@ private:
             return ReadonlyBytes {};
 
         auto fillable_slice = m_buffer.span().slice(m_buffered_size);
-        auto nread = TRY(m_stream.read(fillable_slice));
+        auto nread = TRY(stream().read(fillable_slice));
         m_buffered_size += nread;
         return fillable_slice.slice(0, nread);
     }
 
-    T m_stream;
+    NonnullOwnPtr<T> m_stream;
     // FIXME: Replacing this with a circular buffer would be really nice and
     //        would avoid excessive copies; however, right now
     //        AK::CircularDuplexBuffer inlines its entire contents, and that
@@ -686,7 +686,7 @@ class BufferedSeekable final : public SeekableStream {
     friend BufferedHelper<T>;
 
 public:
-    static ErrorOr<BufferedSeekable<T>> create(T&& stream, size_t buffer_size = 16384)
+    static ErrorOr<NonnullOwnPtr<BufferedSeekable<T>>> create(NonnullOwnPtr<T> stream, size_t buffer_size = 16384)
     {
         return BufferedHelper<T>::template create_buffered<BufferedSeekable>(move(stream), buffer_size);
     }
@@ -719,7 +719,7 @@ public:
     virtual ~BufferedSeekable() override { }
 
 private:
-    BufferedSeekable(T stream, ByteBuffer buffer)
+    BufferedSeekable(NonnullOwnPtr<T> stream, ByteBuffer buffer)
         : m_helper(Badge<BufferedSeekable<T>> {}, move(stream), buffer)
     {
     }
@@ -732,7 +732,7 @@ class BufferedSocket final : public Socket {
     friend BufferedHelper<T>;
 
 public:
-    static ErrorOr<BufferedSocket<T>> create(T&& stream, size_t buffer_size = 16384)
+    static ErrorOr<NonnullOwnPtr<BufferedSocket<T>>> create(NonnullOwnPtr<T> stream, size_t buffer_size = 16384)
     {
         return BufferedHelper<T>::template create_buffered<BufferedSocket>(move(stream), buffer_size);
     }
@@ -776,7 +776,7 @@ public:
     virtual ~BufferedSocket() override { }
 
 private:
-    BufferedSocket(T stream, ByteBuffer buffer)
+    BufferedSocket(NonnullOwnPtr<T> stream, ByteBuffer buffer)
         : m_helper(Badge<BufferedSocket<T>> {}, move(stream), buffer)
     {
         setup_notifier();
@@ -804,14 +804,14 @@ using BufferedLocalSocket = BufferedSocket<LocalSocket>;
 template<SocketLike T>
 class BasicReusableSocket final : public ReusableSocket {
 public:
-    static ErrorOr<BasicReusableSocket<T>> connect(String const& host, u16 port)
+    static ErrorOr<NonnullOwnPtr<BasicReusableSocket<T>>> connect(String const& host, u16 port)
     {
-        return BasicReusableSocket { TRY(T::connect(host, port)) };
+        return make<BasicReusableSocket<T>>(TRY(T::connect(host, port)));
     }
 
-    static ErrorOr<BasicReusableSocket<T>> connect(SocketAddress const& address)
+    static ErrorOr<NonnullOwnPtr<BasicReusableSocket<T>>> connect(SocketAddress const& address)
     {
-        return BasicReusableSocket { TRY(T::connect(address)) };
+        return make<BasicReusableSocket<T>>(TRY(T::connect(address)));
     }
 
     virtual bool is_connected() override
@@ -850,12 +850,12 @@ public:
     virtual ErrorOr<void> set_close_on_exec(bool enabled) override { return m_socket.set_close_on_exec(enabled); }
 
 private:
-    BasicReusableSocket(T&& socket)
+    BasicReusableSocket(NonnullOwnPtr<T> socket)
         : m_socket(move(socket))
     {
     }
 
-    T m_socket;
+    NonnullOwnPtr<T> m_socket;
 };
 
 using ReusableTCPSocket = BasicReusableSocket<TCPSocket>;

+ 3 - 3
Userland/Libraries/LibCore/TCPServer.cpp

@@ -69,7 +69,7 @@ ErrorOr<void> TCPServer::set_blocking(bool blocking)
     return {};
 }
 
-ErrorOr<Stream::TCPSocket> TCPServer::accept()
+ErrorOr<NonnullOwnPtr<Stream::TCPSocket>> TCPServer::accept()
 {
     VERIFY(m_listening);
     sockaddr_in in;
@@ -86,8 +86,8 @@ ErrorOr<Stream::TCPSocket> TCPServer::accept()
     // FIXME: Ideally, we should let the caller decide whether it wants the
     //        socket to be nonblocking or not, but there are currently places
     //        which depend on this.
-    TRY(socket.set_blocking(false));
-    TRY(socket.set_close_on_exec(true));
+    TRY(socket->set_blocking(false));
+    TRY(socket->set_close_on_exec(true));
 #endif
 
     return socket;

+ 1 - 1
Userland/Libraries/LibCore/TCPServer.h

@@ -24,7 +24,7 @@ public:
     ErrorOr<void> listen(IPv4Address const& address, u16 port);
     ErrorOr<void> set_blocking(bool blocking);
 
-    ErrorOr<Stream::TCPSocket> accept();
+    ErrorOr<NonnullOwnPtr<Stream::TCPSocket>> accept();
 
     Optional<IPv4Address> local_address() const;
     Optional<u16> local_port() const;

+ 8 - 8
Userland/Services/EchoServer/Client.cpp

@@ -7,12 +7,12 @@
 #include "Client.h"
 #include "LibCore/EventLoop.h"
 
-Client::Client(int id, Core::Stream::TCPSocket socket)
+Client::Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket)
     : m_id(id)
     , m_socket(move(socket))
 {
-    m_socket.on_ready_to_read = [this] {
-        if (m_socket.is_eof())
+    m_socket->on_ready_to_read = [this] {
+        if (m_socket->is_eof())
             return;
 
         auto result = drain_socket();
@@ -32,17 +32,17 @@ ErrorOr<void> Client::drain_socket()
         return ENOMEM;
     auto buffer = maybe_buffer.release_value();
 
-    while (TRY(m_socket.can_read_without_blocking())) {
-        auto nread = TRY(m_socket.read(buffer));
+    while (TRY(m_socket->can_read_without_blocking())) {
+        auto nread = TRY(m_socket->read(buffer));
 
         dbgln("Read {} bytes.", nread);
 
-        if (m_socket.is_eof()) {
+        if (m_socket->is_eof()) {
             Core::deferred_invoke([this, strong_this = NonnullRefPtr(*this)] { quit(); });
             break;
         }
 
-        TRY(m_socket.write({ buffer.data(), nread }));
+        TRY(m_socket->write({ buffer.data(), nread }));
     }
 
     return {};
@@ -50,7 +50,7 @@ ErrorOr<void> Client::drain_socket()
 
 void Client::quit()
 {
-    m_socket.close();
+    m_socket->close();
     if (on_exit)
         on_exit();
 }

+ 3 - 3
Userland/Services/EchoServer/Client.h

@@ -10,7 +10,7 @@
 
 class Client : public RefCounted<Client> {
 public:
-    static NonnullRefPtr<Client> create(int id, Core::Stream::TCPSocket socket)
+    static NonnullRefPtr<Client> create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket)
     {
         return adopt_ref(*new Client(id, move(socket)));
     }
@@ -18,12 +18,12 @@ public:
     Function<void()> on_exit;
 
 protected:
-    Client(int id, Core::Stream::TCPSocket socket);
+    Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket);
 
     ErrorOr<void> drain_socket();
     void quit();
 
 private:
     int m_id { 0 };
-    Core::Stream::TCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::TCPSocket> m_socket;
 };

+ 11 - 10
Userland/Services/TelnetServer/Client.cpp

@@ -5,6 +5,7 @@
  */
 
 #include "Client.h"
+
 #include <AK/ByteBuffer.h>
 #include <AK/MemoryStream.h>
 #include <AK/String.h>
@@ -16,13 +17,13 @@
 #include <stdio.h>
 #include <unistd.h>
 
-Client::Client(int id, Core::Stream::TCPSocket socket, int ptm_fd)
+Client::Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd)
     : m_id(id)
     , m_socket(move(socket))
     , m_ptm_fd(ptm_fd)
     , m_ptm_notifier(Core::Notifier::construct(ptm_fd, Core::Notifier::Read))
 {
-    m_socket.on_ready_to_read = [this] {
+    m_socket->on_ready_to_read = [this] {
         auto result = drain_socket();
         if (result.is_error()) {
             dbgln("Failed to drain the socket: {}", result.error());
@@ -50,7 +51,7 @@ Client::Client(int id, Core::Stream::TCPSocket socket, int ptm_fd)
     m_parser.on_error = [this]() { handle_error(); };
 }
 
-ErrorOr<NonnullRefPtr<Client>> Client::create(int id, Core::Stream::TCPSocket socket, int ptm_fd)
+ErrorOr<NonnullRefPtr<Client>> Client::create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd)
 {
     auto client = adopt_ref(*new Client(id, move(socket), ptm_fd));
 
@@ -77,12 +78,12 @@ ErrorOr<void> Client::drain_socket()
         return ENOMEM;
     auto buffer = maybe_buffer.release_value();
 
-    while (TRY(m_socket.can_read_without_blocking())) {
-        auto nread = TRY(m_socket.read(buffer));
+    while (TRY(m_socket->can_read_without_blocking())) {
+        auto nread = TRY(m_socket->read(buffer));
 
         m_parser.write({ buffer.data(), nread });
 
-        if (m_socket.is_eof()) {
+        if (m_socket->is_eof()) {
             Core::deferred_invoke([this, strong_this = NonnullRefPtr(*this)] { quit(); });
             break;
         }
@@ -162,7 +163,7 @@ ErrorOr<void> Client::send_data(StringView data)
     }
 
     if (fast) {
-        TRY(m_socket.write({ data.characters_without_null_termination(), data.length() }));
+        TRY(m_socket->write({ data.characters_without_null_termination(), data.length() }));
         return {};
     }
 
@@ -184,7 +185,7 @@ ErrorOr<void> Client::send_data(StringView data)
     }
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
     return {};
 }
 
@@ -205,7 +206,7 @@ ErrorOr<void> Client::send_commands(Vector<Command> commands)
         stream << (u8)IAC << command.command << command.subcommand;
 
     VERIFY(stream.is_end());
-    TRY(m_socket.write({ buffer.data(), buffer.size() }));
+    TRY(m_socket->write({ buffer.data(), buffer.size() }));
     return {};
 }
 
@@ -213,7 +214,7 @@ void Client::quit()
 {
     m_ptm_notifier->set_enabled(false);
     close(m_ptm_fd);
-    m_socket.close();
+    m_socket->close();
     if (on_exit)
         on_exit();
 }

+ 3 - 3
Userland/Services/TelnetServer/Client.h

@@ -17,12 +17,12 @@
 
 class Client : public RefCounted<Client> {
 public:
-    static ErrorOr<NonnullRefPtr<Client>> create(int id, Core::Stream::TCPSocket socket, int ptm_fd);
+    static ErrorOr<NonnullRefPtr<Client>> create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd);
 
     Function<void()> on_exit;
 
 private:
-    Client(int id, Core::Stream::TCPSocket socket, int ptm_fd);
+    Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd);
 
     ErrorOr<void> drain_socket();
     ErrorOr<void> drain_pty();
@@ -38,7 +38,7 @@ private:
     // client id
     int m_id { 0 };
     // client resources
-    Core::Stream::TCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::TCPSocket> m_socket;
     Parser m_parser;
     // pty resources
     int m_ptm_fd { -1 };

+ 4 - 4
Userland/Services/TelnetServer/main.cpp

@@ -106,7 +106,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     server->on_ready_to_accept = [&next_id, &clients, &server, command] {
         int id = next_id++;
 
-        ErrorOr<Core::Stream::TCPSocket> maybe_client_socket = server->accept();
+        auto maybe_client_socket = server->accept();
         if (maybe_client_socket.is_error()) {
             warnln("accept: {}", maybe_client_socket.error());
             return;
@@ -116,17 +116,17 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         int ptm_fd = posix_openpt(O_RDWR);
         if (ptm_fd < 0) {
             perror("posix_openpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
         if (grantpt(ptm_fd) < 0) {
             perror("grantpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
         if (unlockpt(ptm_fd) < 0) {
             perror("unlockpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
 

+ 11 - 11
Userland/Services/WebServer/Client.cpp

@@ -28,7 +28,7 @@
 
 namespace WebServer {
 
-Client::Client(Core::Stream::BufferedTCPSocket socket, Core::Object* parent)
+Client::Client(NonnullOwnPtr<Core::Stream::BufferedTCPSocket> socket, Core::Object* parent)
     : Core::Object(parent)
     , m_socket(move(socket))
 {
@@ -36,16 +36,16 @@ Client::Client(Core::Stream::BufferedTCPSocket socket, Core::Object* parent)
 
 void Client::die()
 {
-    m_socket.close();
+    m_socket->close();
     deferred_invoke([this] { remove_from_parent(); });
 }
 
 void Client::start()
 {
-    m_socket.on_ready_to_read = [this] {
+    m_socket->on_ready_to_read = [this] {
         StringBuilder builder;
 
-        auto maybe_buffer = ByteBuffer::create_uninitialized(m_socket.buffer_size());
+        auto maybe_buffer = ByteBuffer::create_uninitialized(m_socket->buffer_size());
         if (!maybe_buffer.has_value()) {
             warnln("Could not create buffer for client (possibly out of memory)");
             die();
@@ -54,7 +54,7 @@ void Client::start()
 
         auto buffer = maybe_buffer.release_value();
         for (;;) {
-            auto maybe_can_read = m_socket.can_read_without_blocking();
+            auto maybe_can_read = m_socket->can_read_without_blocking();
             if (maybe_can_read.is_error()) {
                 warnln("Failed to get the blocking status for the socket: {}", maybe_can_read.error());
                 die();
@@ -64,14 +64,14 @@ void Client::start()
             if (!maybe_can_read.value())
                 break;
 
-            auto maybe_nread = m_socket.read_until_any_of(buffer, Array { "\r"sv, "\n"sv, "\r\n"sv });
+            auto maybe_nread = m_socket->read_until_any_of(buffer, Array { "\r"sv, "\n"sv, "\r\n"sv });
             if (maybe_nread.is_error()) {
                 warnln("Failed to read a line from the request: {}", maybe_nread.error());
                 die();
                 return;
             }
 
-            if (m_socket.is_eof()) {
+            if (m_socket->is_eof()) {
                 die();
                 break;
             }
@@ -182,7 +182,7 @@ ErrorOr<void> Client::send_response(InputStream& response, HTTP::HttpRequest con
     builder.append("\r\n");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
     log_response(200, request);
 
     char buffer[PAGE_SIZE];
@@ -193,7 +193,7 @@ ErrorOr<void> Client::send_response(InputStream& response, HTTP::HttpRequest con
 
         ReadonlyBytes write_buffer { buffer, size };
         while (!write_buffer.is_empty()) {
-            auto nwritten = TRY(m_socket.write(write_buffer));
+            auto nwritten = TRY(m_socket->write(write_buffer));
 
             if (nwritten == 0) {
                 dbgln("EEEEEE got 0 bytes written!");
@@ -216,7 +216,7 @@ ErrorOr<void> Client::send_redirect(StringView redirect_path, HTTP::HttpRequest
     builder.append("\r\n");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
 
     log_response(301, request);
     return {};
@@ -341,7 +341,7 @@ ErrorOr<void> Client::send_error_response(unsigned code, HTTP::HttpRequest const
     builder.append("</h1></body></html>");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
 
     log_response(code, request);
     return {};

+ 2 - 2
Userland/Services/WebServer/Client.h

@@ -20,7 +20,7 @@ public:
     void start();
 
 private:
-    Client(Core::Stream::BufferedTCPSocket, Core::Object* parent);
+    Client(NonnullOwnPtr<Core::Stream::BufferedTCPSocket>, Core::Object* parent);
 
     ErrorOr<bool> handle_request(ReadonlyBytes);
     ErrorOr<void> send_response(InputStream&, HTTP::HttpRequest const&, String const& content_type);
@@ -31,7 +31,7 @@ private:
     ErrorOr<void> handle_directory_listing(String const& requested_path, String const& real_path, HTTP::HttpRequest const&);
     bool verify_credentials(Vector<HTTP::HttpRequest::Header> const&);
 
-    Core::Stream::BufferedTCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::BufferedTCPSocket> m_socket;
 };
 
 }

+ 1 - 1
Userland/Services/WebServer/main.cpp

@@ -85,7 +85,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         }
 
         // FIXME: Propagate errors
-        MUST(maybe_buffered_socket.value().set_blocking(true));
+        MUST(maybe_buffered_socket.value()->set_blocking(true));
         auto client = WebServer::Client::construct(maybe_buffered_socket.release_value(), server);
         client->start();
     };