Store raw pointer in udp::peer and implement copy/move-assignment (#403)

* Store raw pointer in udp::peer and implement copy/move-assignment

Previously the peer struct held a reference member that could not be
reseated and so the defaulted copy/move-assignments were implicitly
deleted. We want these objects to be moveable and copyable so instead store
a raw pointer.

* Remove potential for false-positive fail in task_container test

Since we detach our (fast to run) task, it might complete on the
thread_pool thread before we get time-sliced in to check its size on the
main thread, leading to a false positive failure in that the container is
already empty.

To fix this, use an event to ensure the task does not complete until we've
checked the container's size.

Author: wence-

include/coro/net/udp/peer.hpp

@@ -10,7 +10,6 @@
 
 #include <chrono>
 #include <span>
-#include <variant>
 
 namespace coro
 {
@@ -43,11 +42,11 @@ class peer
      */
     explicit peer(std::unique_ptr<coro::io_scheduler>& scheduler, const info& bind_info);
 
-    peer(const peer&)                             = default;
-    peer(peer&&)                                  = default;
-    auto operator=(const peer&) noexcept -> peer& = default;
-    auto operator=(peer&&) noexcept -> peer&      = default;
-    ~peer()                                       = default;
+    peer(const peer&) noexcept = default;
+    peer(peer&&) noexcept;
+    auto operator=(const peer&) noexcept -> peer&;
+    auto operator=(peer&&) noexcept -> peer&;
+    ~peer() = default;
 
     /**
      * @return A reference to the underlying socket.
@@ -147,7 +146,7 @@ class peer
 
 private:
     /// The scheduler that will drive this udp client.
-    std::unique_ptr<coro::io_scheduler>& m_io_scheduler;
+    coro::io_scheduler* m_io_scheduler;
     /// The udp socket.
     net::socket m_socket{-1};
     /// Did the user request this udp socket is bound locally to receive packets?

src/net/udp/peer.cpp

@@ -1,21 +1,60 @@
 #include "coro/net/udp/peer.hpp"
+#include <memory>
+#include <utility>
 
 namespace coro::net::udp
 {
 peer::peer(std::unique_ptr<coro::io_scheduler>& scheduler, net::domain_t domain)
-    : m_io_scheduler(scheduler),
+    : m_io_scheduler(scheduler.get()),
       m_socket(net::make_socket(net::socket::options{domain, net::socket::type_t::udp, net::socket::blocking_t::no}))
 {
+    if (m_io_scheduler == nullptr)
+    {
+        throw std::runtime_error("udp::peer cannot have nullptr scheduler");
+    }
 }
 
 peer::peer(std::unique_ptr<coro::io_scheduler>& scheduler, const info& bind_info)
-    : m_io_scheduler(scheduler),
-      m_socket(net::make_accept_socket(
-          net::socket::options{bind_info.address.domain(), net::socket::type_t::udp, net::socket::blocking_t::no},
-          bind_info.address,
-          bind_info.port)),
+    : m_io_scheduler(scheduler.get()),
+      m_socket(
+          net::make_accept_socket(
+              net::socket::options{bind_info.address.domain(), net::socket::type_t::udp, net::socket::blocking_t::no},
+              bind_info.address,
+              bind_info.port)),
       m_bound(true)
 {
+    if (m_io_scheduler == nullptr)
+    {
+        throw std::runtime_error("udp::peer cannot have nullptr scheduler");
+    }
 }
 
+peer::peer(peer&& other) noexcept
+    : m_io_scheduler(std::exchange(other.m_io_scheduler, nullptr)),
+      m_socket(std::move(other.m_socket)),
+      m_bound(other.m_bound)
+{
+}
+
+auto peer::operator=(peer&& other) noexcept -> peer&
+{
+    if (std::addressof(other) != this)
+    {
+        m_io_scheduler = std::exchange(other.m_io_scheduler, nullptr);
+        m_socket       = std::move(other.m_socket);
+        m_bound        = other.m_bound;
+    }
+    return *this;
+}
+
+auto peer::operator=(const peer& other) noexcept -> peer&
+{
+    if (std::addressof(other) != this)
+    {
+        m_io_scheduler = other.m_io_scheduler;
+        m_socket       = other.m_socket;
+        m_bound        = other.m_bound;
+    }
+    return *this;
+}
 } // namespace coro::net::udp

test/test_task_container.cpp

@@ -14,19 +14,22 @@ using namespace std::chrono_literals;
 
 TEST_CASE("task_container schedule single task", "[task_container]")
 {
-    auto s = coro::thread_pool::make_unique(
-        coro::thread_pool::options{.thread_count = 1});
-
-    int value = 0;
-    auto make_task = [] (int &value) -> coro::task<void>
+    auto                    s = coro::thread_pool::make_unique(coro::thread_pool::options{.thread_count = 1});
+    coro::event             can_start{false};
+    int                     value = 0;
+    auto make_task = [](int& value, coro::event& can_start) -> coro::task<void>
     {
+        co_await can_start;
         value = 37;
         co_return;
     };
 
     coro::task_container<coro::thread_pool> tc{s};
-    tc.start(make_task(value));
+    tc.start(make_task(value, can_start));
     REQUIRE(tc.size() == 1);
+    // Only allow the task to complete once we have checked the container's size,
+    // otherwise the require could "spuriously" fail due to the task already being done.
+    can_start.set(s);
     coro::sync_wait(tc.yield_until_empty());
     REQUIRE(value == 37);
     REQUIRE(tc.empty());