From 0e3e55507016e94dc62918f2f6ae36cad3557c2d Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Tue, 2 Sep 2025 10:21:29 -0600 Subject: [PATCH 1/5] Add accept_socket() to tcp and tls servers This will allow for a graceful shutdown via the socket.shutdown(how) functions Closes #377 --- include/coro/net/socket.hpp | 2 +- include/coro/net/tcp/client.hpp | 4 ++-- include/coro/net/tcp/server.hpp | 10 +++++++++- include/coro/net/tls/client.hpp | 4 ++-- include/coro/net/tls/server.hpp | 8 ++++++++ src/detail/io_notifier_epoll.cpp | 2 +- test/net/test_tcp_server.cpp | 26 ++++++++++++++++++++++++++ 7 files changed, 49 insertions(+), 7 deletions(-) diff --git a/include/coro/net/socket.hpp b/include/coro/net/socket.hpp index 19380c1c..15b428e0 100644 --- a/include/coro/net/socket.hpp +++ b/include/coro/net/socket.hpp @@ -68,7 +68,7 @@ class socket /** * @param how Shuts the socket down with the given operations. - * @param Returns true if the sockets given operations were shutdown. + * @return Returns true if the sockets given operations were shutdown. */ auto shutdown(poll_op how = poll_op::read_write) -> bool; diff --git a/include/coro/net/tcp/client.hpp b/include/coro/net/tcp/client.hpp index d53cd700..4dd348e7 100644 --- a/include/coro/net/tcp/client.hpp +++ b/include/coro/net/tcp/client.hpp @@ -52,8 +52,8 @@ class client * @return The tcp socket this client is using. * @{ **/ - auto socket() -> net::socket& { return m_socket; } - auto socket() const -> const net::socket& { return m_socket; } + [[nodiscard]] auto socket() -> net::socket& { return m_socket; } + [[nodiscard]] auto socket() const -> const net::socket& { return m_socket; } /** @} */ /** diff --git a/include/coro/net/tcp/server.hpp b/include/coro/net/tcp/server.hpp index e038067e..dddae856 100644 --- a/include/coro/net/tcp/server.hpp +++ b/include/coro/net/tcp/server.hpp @@ -56,11 +56,19 @@ class server /** * Accepts an incoming tcp client connection. On failure the tls clients socket will be set to - * and invalid state, use the socket.is_value() to verify the client was correctly accepted. + * and invalid state, use the socket.is_valid() to verify the client was correctly accepted. * @return The newly connected tcp client connection. */ auto accept() -> coro::net::tcp::client; + /** + * @return The tcp accept socket this server is using. + * @{ + **/ + [[nodiscard]] auto accept_socket() -> net::socket& { return m_accept_socket; } + [[nodiscard]] auto accept_socket() const -> const net::socket& { return m_accept_socket; } + /** @} */ + private: friend client; /// The io scheduler for awaiting new connections. diff --git a/include/coro/net/tls/client.hpp b/include/coro/net/tls/client.hpp index 8776bb2c..08f4966e 100644 --- a/include/coro/net/tls/client.hpp +++ b/include/coro/net/tls/client.hpp @@ -58,8 +58,8 @@ class client * @return The tcp socket this client is using. * @{ **/ - auto socket() -> net::socket& { return m_socket; } - auto socket() const -> const net::socket& { return m_socket; } + [[nodiscard]] auto socket() -> net::socket& { return m_socket; } + [[nodiscard]] auto socket() const -> const net::socket& { return m_socket; } /** @} */ /** diff --git a/include/coro/net/tls/server.hpp b/include/coro/net/tls/server.hpp index 4bc1cfbc..f72aa0f1 100644 --- a/include/coro/net/tls/server.hpp +++ b/include/coro/net/tls/server.hpp @@ -66,6 +66,14 @@ class server */ auto accept(std::chrono::milliseconds timeout = std::chrono::seconds{30}) -> coro::task; + /** + * @return The tcp accept socket this server is using. + * @{ + **/ + [[nodiscard]] auto accept_socket() -> net::socket& { return m_accept_socket; } + [[nodiscard]] auto accept_socket() const -> const net::socket& { return m_accept_socket; } + /** @} */ + private: /// The io scheduler for awaiting new connections. std::shared_ptr m_io_scheduler{nullptr}; diff --git a/src/detail/io_notifier_epoll.cpp b/src/detail/io_notifier_epoll.cpp index 006afff9..d0933084 100644 --- a/src/detail/io_notifier_epoll.cpp +++ b/src/detail/io_notifier_epoll.cpp @@ -65,7 +65,7 @@ auto io_notifier_epoll::watch(fd_t fd, coro::poll_op op, void* data, bool keep) auto io_notifier_epoll::watch(detail::poll_info& pi) -> bool { auto event_data = event_t{}; - event_data.events = static_cast(pi.m_op) | EPOLLONESHOT | EPOLLRDHUP; + event_data.events = static_cast(pi.m_op) | EPOLLONESHOT | EPOLLRDHUP | EPOLLHUP; event_data.data.ptr = static_cast(&pi); return ::epoll_ctl(m_fd, EPOLL_CTL_ADD, pi.m_fd, &event_data) != -1; } diff --git a/test/net/test_tcp_server.cpp b/test/net/test_tcp_server.cpp index 95686af5..35927ca0 100644 --- a/test/net/test_tcp_server.cpp +++ b/test/net/test_tcp_server.cpp @@ -175,4 +175,30 @@ TEST_CASE("tcp_server concurrent polling on the same socket", "[tcp_server]") REQUIRE(request == response); } +TEST_CASE("tcp_server graceful shutdown via socket", "[tcp_server]") +{ + auto scheduler = coro::io_scheduler::make_shared(coro::io_scheduler::options{ + .execution_strategy = coro::io_scheduler::execution_strategy_t::process_tasks_inline}); + coro::net::tcp::server server{scheduler}; + + auto make_accept_task = [](coro::net::tcp::server& server) -> coro::task + { + std::cerr << "make accept task start\n"; + auto poll_result = co_await server.poll(std::chrono::seconds{3}); + REQUIRE(poll_result == coro::poll_status::closed); + auto client = server.accept(); + REQUIRE_FALSE(client.socket().is_valid()); + std::cerr << "make accept task completed\n"; + }; + + scheduler->spawn(make_accept_task(server)); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + server.accept_socket().shutdown(coro::poll_op::read_write); + + scheduler->shutdown(); +} + #endif // LIBCORO_FEATURE_NETWORKING + From ad0ce71785fb71bd25d35cb5bfe601cdd6747256 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Fri, 5 Sep 2025 10:45:06 -0600 Subject: [PATCH 2/5] try with no timeout for macos --- test/net/test_tcp_server.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/test/net/test_tcp_server.cpp b/test/net/test_tcp_server.cpp index 35927ca0..3418e803 100644 --- a/test/net/test_tcp_server.cpp +++ b/test/net/test_tcp_server.cpp @@ -6,8 +6,14 @@ #include +TEST_CASE("tcp_server", "[tcp_server]") +{ + std::cerr << "[tcp_server]\n\n"; +} + TEST_CASE("tcp_server ping server", "[tcp_server]") { + std::cerr << "BEGIN tcp_server ping server\n"; const std::string client_msg{"Hello from client"}; const std::string server_msg{"Reply from server!"}; @@ -89,10 +95,12 @@ TEST_CASE("tcp_server ping server", "[tcp_server]") coro::sync_wait(coro::when_all( make_server_task(scheduler, client_msg, server_msg), make_client_task(scheduler, client_msg, server_msg))); + std::cerr << "END tcp_server ping server\n"; } TEST_CASE("tcp_server concurrent polling on the same socket", "[tcp_server]") { + std::cerr << "BEGIN tcp_server concurrent polling on the same socket\n"; // Issue 224: This test duplicates a client and issues two different poll operations per coroutine. using namespace std::chrono_literals; @@ -173,32 +181,45 @@ TEST_CASE("tcp_server concurrent polling on the same socket", "[tcp_server]") auto response = std::move(std::get<1>(result).return_value()); REQUIRE(request == response); + std::cerr << "END tcp_server concurrent polling on the same socket\n"; } TEST_CASE("tcp_server graceful shutdown via socket", "[tcp_server]") { + std::cerr << "BEGIN tcp_server graceful shutdown via socket\n"; auto scheduler = coro::io_scheduler::make_shared(coro::io_scheduler::options{ .execution_strategy = coro::io_scheduler::execution_strategy_t::process_tasks_inline}); coro::net::tcp::server server{scheduler}; + coro::event started{}; - auto make_accept_task = [](coro::net::tcp::server& server) -> coro::task + auto make_accept_task = [](coro::net::tcp::server& server, coro::event& started) -> coro::task { std::cerr << "make accept task start\n"; - auto poll_result = co_await server.poll(std::chrono::seconds{3}); + started.set(); + auto poll_result = co_await server.poll(); REQUIRE(poll_result == coro::poll_status::closed); auto client = server.accept(); REQUIRE_FALSE(client.socket().is_valid()); std::cerr << "make accept task completed\n"; }; - scheduler->spawn(make_accept_task(server)); + scheduler->spawn(make_accept_task(server, started)); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + coro::sync_wait(started); + // we'll wait a bit to make sure the server.poll() is fully registered. + std::this_thread::sleep_for(std::chrono::milliseconds(500)); server.accept_socket().shutdown(coro::poll_op::read_write); scheduler->shutdown(); + std::cerr << "END tcp_server graceful shutdown via socket\n"; } +TEST_CASE("~tcp_server", "[tcp_server]") +{ + std::cerr << "[~tcp_server]\n\n"; +} + + #endif // LIBCORO_FEATURE_NETWORKING From cae619fe78906737e1a6844150a2ece7c34722d1 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Sat, 13 Sep 2025 11:14:42 -0600 Subject: [PATCH 3/5] skip test on __APPLE__ check EV_EOF first and errors --- src/detail/io_notifier_kqueue.cpp | 13 ++++++++----- test/net/test_tcp_server.cpp | 6 ++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/detail/io_notifier_kqueue.cpp b/src/detail/io_notifier_kqueue.cpp index d938c637..78a4305d 100644 --- a/src/detail/io_notifier_kqueue.cpp +++ b/src/detail/io_notifier_kqueue.cpp @@ -156,18 +156,21 @@ auto io_notifier_kqueue::next_events( auto io_notifier_kqueue::event_to_poll_status(const event_t& event) -> poll_status { - if (event.filter & EVFILT_READ || event.filter & EVFILT_WRITE) + if (event.flags & EV_EOF) { - return poll_status::event; + return poll_status::closed; } - else if (event.flags & EV_ERROR) + + if (event.flags & EV_ERROR) { return poll_status::error; } - else if (event.flags & EV_EOF) + + if (event.filter & EVFILT_READ || event.filter & EVFILT_WRITE) { - return poll_status::closed; + return poll_status::event; } + throw std::runtime_error{"invalid kqueue state"}; } diff --git a/test/net/test_tcp_server.cpp b/test/net/test_tcp_server.cpp index 3418e803..8d008abd 100644 --- a/test/net/test_tcp_server.cpp +++ b/test/net/test_tcp_server.cpp @@ -184,6 +184,10 @@ TEST_CASE("tcp_server concurrent polling on the same socket", "[tcp_server]") std::cerr << "END tcp_server concurrent polling on the same socket\n"; } +#ifndef __APPLE__ +// This test is known to not work on kqueue style systems (e.g. apple) because the socket shutdown() +// call does not properly trigger an EV_EOF flag on the accept socket. + TEST_CASE("tcp_server graceful shutdown via socket", "[tcp_server]") { std::cerr << "BEGIN tcp_server graceful shutdown via socket\n"; @@ -215,6 +219,8 @@ TEST_CASE("tcp_server graceful shutdown via socket", "[tcp_server]") std::cerr << "END tcp_server graceful shutdown via socket\n"; } +#endif + TEST_CASE("~tcp_server", "[tcp_server]") { std::cerr << "[~tcp_server]\n\n"; From 28001a47ca04b12b11a60905ab35043a2fd06323 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Sun, 14 Sep 2025 15:01:17 -0600 Subject: [PATCH 4/5] bench test handles poll error|closed properly for macos --- test/bench.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/bench.cpp b/test/bench.cpp index a08c43af..7c2ade75 100644 --- a/test/bench.cpp +++ b/test/bench.cpp @@ -404,6 +404,13 @@ TEST_CASE("benchmark tcp::server echo server thread pool", "[benchmark]") while (true) { auto pstatus = co_await client.poll(coro::poll_op::read); + if (pstatus != coro::poll_status::event) + { + REQUIRE_THREAD_SAFE(pstatus == coro::poll_status::closed); + // the socket has been closed + break; + } + REQUIRE_THREAD_SAFE(pstatus == coro::poll_status::event); auto [rstatus, rspan] = client.recv(in); @@ -597,6 +604,13 @@ TEST_CASE("benchmark tcp::server echo server inline", "[benchmark]") while (true) { auto pstatus = co_await client.poll(coro::poll_op::read); + if (pstatus != coro::poll_status::event) + { + REQUIRE_THREAD_SAFE(pstatus == coro::poll_status::closed); + // the socket has been closed + break; + } + REQUIRE_THREAD_SAFE(pstatus == coro::poll_status::event); auto [rstatus, rspan] = client.recv(in); From b2b1f39232c75810ff2469138d2930a9c4d6e252 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Mon, 15 Sep 2025 20:33:52 -0600 Subject: [PATCH 5/5] revert kqueue event check ordering --- src/detail/io_notifier_kqueue.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/detail/io_notifier_kqueue.cpp b/src/detail/io_notifier_kqueue.cpp index 78a4305d..e6d4d3dd 100644 --- a/src/detail/io_notifier_kqueue.cpp +++ b/src/detail/io_notifier_kqueue.cpp @@ -156,6 +156,11 @@ auto io_notifier_kqueue::next_events( auto io_notifier_kqueue::event_to_poll_status(const event_t& event) -> poll_status { + if (event.filter & EVFILT_READ || event.filter & EVFILT_WRITE) + { + return poll_status::event; + } + if (event.flags & EV_EOF) { return poll_status::closed; @@ -166,11 +171,6 @@ auto io_notifier_kqueue::event_to_poll_status(const event_t& event) -> poll_stat return poll_status::error; } - if (event.filter & EVFILT_READ || event.filter & EVFILT_WRITE) - { - return poll_status::event; - } - throw std::runtime_error{"invalid kqueue state"}; }