From 816035627f7786150e942aad6c0652bb304e3831 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Fri, 14 Jun 2024 13:57:28 -0600 Subject: [PATCH 1/3] lift::client destructor infinite loop with libuv >= 1.48 * libuv >= 1.48 at least has changed how uv loops get shutdown * The loop seems to need to release its own resources * Introduced a new async channel for shutdown for the loop to cleanup its own uv_handle_t objects. Closes #149 --- inc/lift/client.hpp | 10 +++++++--- src/client.cpp | 34 ++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/inc/lift/client.hpp b/inc/lift/client.hpp index 5878433..5f77e47 100644 --- a/inc/lift/client.hpp +++ b/inc/lift/client.hpp @@ -77,10 +77,10 @@ class client ~client(); - client(const client&) = delete; - client(client&&) = delete; + client(const client&) = delete; + client(client&&) = delete; auto operator=(const client&) noexcept -> client& = delete; - auto operator=(client&&) noexcept -> client& = delete; + auto operator=(client&&) noexcept -> client& = delete; /** * @return True if the event loop is currently running. @@ -224,6 +224,8 @@ class client uv_loop_t m_uv_loop{}; /// The async trigger for injecting new requests into the event loop. uv_async_t m_uv_async{}; + /// The async trigger to let uv_run() know its being shutdown + uv_async_t m_uv_async_shutdown_pipe{}; /// libcurl requires a single timer to drive internal timeouts/wake-ups. uv_timer_t m_uv_timer_curl{}; /// If set, the amount of time connections are allowed to connect, this can be @@ -477,6 +479,8 @@ class client */ friend auto on_uv_requests_accept_async(uv_async_t* handle) -> void; + friend auto on_uv_shutdown_async(uv_async_t* handle) -> void; + friend auto on_uv_timesup_callback(uv_timer_t* handle) -> void; }; diff --git a/src/client.cpp b/src/client.cpp index 647af0b..50d2a3d 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -27,10 +27,10 @@ class curl_context ~curl_context() = default; - curl_context(const curl_context&) = delete; - curl_context(curl_context&&) = delete; + curl_context(const curl_context&) = delete; + curl_context(curl_context&&) = delete; auto operator=(const curl_context&) noexcept -> curl_context& = delete; - auto operator=(curl_context&&) noexcept -> curl_context& = delete; + auto operator=(curl_context&&) noexcept -> curl_context& = delete; auto init(uv_loop_t* uv_loop, curl_socket_t sock_fd) -> void { @@ -79,6 +79,8 @@ auto on_uv_curl_perform_callback(uv_poll_t* req, int status, int events) -> void auto on_uv_requests_accept_async(uv_async_t* handle) -> void; +auto on_uv_shutdown_async(uv_async_t* handle) -> void; + auto on_uv_timesup_callback(uv_timer_t* handle) -> void; client::client(options opts) @@ -100,6 +102,9 @@ client::client(options opts) uv_async_init(&m_uv_loop, &m_uv_async, on_uv_requests_accept_async); m_uv_async.data = this; + uv_async_init(&m_uv_loop, &m_uv_async_shutdown_pipe, on_uv_shutdown_async); + m_uv_async_shutdown_pipe.data = this; + uv_timer_init(&m_uv_loop, &m_uv_timer_curl); m_uv_timer_curl.data = this; @@ -133,23 +138,20 @@ client::~client() { m_is_stopping.exchange(true, std::memory_order_release); + // Block until all requests are completed. while (!empty()) { std::this_thread::sleep_for(1ms); } - uv_timer_stop(&m_uv_timer_curl); - uv_timer_stop(&m_uv_timer_timeout); - uv_close(uv_type_cast(&m_uv_timer_curl), uv_close_callback); - uv_close(uv_type_cast(&m_uv_timer_timeout), uv_close_callback); - uv_close(uv_type_cast(&m_uv_async), uv_close_callback); + uv_async_send(&m_uv_async_shutdown_pipe); + uv_stop(&m_uv_loop); while (uv_loop_alive(&m_uv_loop)) { std::this_thread::sleep_for(1ms); - uv_async_send(&m_uv_async); // fake a request to make sure the loop wakes up } - uv_stop(&m_uv_loop); + uv_loop_close(&m_uv_loop); m_background_thread.join(); @@ -663,6 +665,18 @@ auto on_uv_requests_accept_async(uv_async_t* handle) -> void c->m_grabbed_requests.clear(); } +auto on_uv_shutdown_async(uv_async_t* handle) -> void +{ + auto* c = static_cast(handle->data); + + uv_timer_stop(&c->m_uv_timer_curl); + uv_timer_stop(&c->m_uv_timer_timeout); + uv_close(uv_type_cast(&c->m_uv_timer_curl), uv_close_callback); + uv_close(uv_type_cast(&c->m_uv_timer_timeout), uv_close_callback); + uv_close(uv_type_cast(&c->m_uv_async), uv_close_callback); + uv_close(uv_type_cast(&c->m_uv_async_shutdown_pipe), uv_close_callback); +} + auto on_uv_timesup_callback(uv_timer_t* handle) -> void { auto* c = static_cast(handle->data); From 229adb47a1b4e5324b19673721e8355d930bd337 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Fri, 14 Jun 2024 14:19:55 -0600 Subject: [PATCH 2/3] Fix for libuv < 1.48 --- src/client.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 50d2a3d..0b1721d 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -144,12 +144,18 @@ client::~client() std::this_thread::sleep_for(1ms); } - uv_async_send(&m_uv_async_shutdown_pipe); uv_stop(&m_uv_loop); + uv_async_send(&m_uv_async_shutdown_pipe); - while (uv_loop_alive(&m_uv_loop)) + while (true) { - std::this_thread::sleep_for(1ms); + int v = uv_loop_alive(&m_uv_loop); + std::cerr << "uv_loop_alive() = [" << v << "]\n"; + if (v == 0) + { + break; + } + std::this_thread::sleep_for(1s); } uv_loop_close(&m_uv_loop); @@ -214,7 +220,15 @@ auto client::run() -> void } m_is_running.exchange(true, std::memory_order_release); - uv_run(&m_uv_loop, UV_RUN_DEFAULT); + int v = uv_run(&m_uv_loop, UV_RUN_DEFAULT); + std::cerr << "uv_run(UV_RUN_DEFAULT) = [" << v << "]\n"; + while (v > 0) + { + v = uv_run(&m_uv_loop, UV_RUN_NOWAIT); + std::cerr << "uv_run(UV_RUN_NOWAIT) = [" << v << "]\n"; + std::this_thread::sleep_for(std::chrono::milliseconds{1}); + } + m_is_running.exchange(false, std::memory_order_release); if (m_on_thread_callback != nullptr) From 8784ae5894ecef4e327d56bf0901bd57d0ed7bc8 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Fri, 14 Jun 2024 15:48:17 -0600 Subject: [PATCH 3/3] Working on fedora, removing sleep + std::cerr --- src/client.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 0b1721d..b3679fc 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -144,18 +144,15 @@ client::~client() std::this_thread::sleep_for(1ms); } + // This breaks the main UV_RUN_DEFAULT loop. uv_stop(&m_uv_loop); + // This tells the loop to cleanup all its resources. uv_async_send(&m_uv_async_shutdown_pipe); - while (true) + // Wait for the loop to finish cleaning up before closing up shop. + while (uv_loop_alive(&m_uv_loop) > 0) { - int v = uv_loop_alive(&m_uv_loop); - std::cerr << "uv_loop_alive() = [" << v << "]\n"; - if (v == 0) - { - break; - } - std::this_thread::sleep_for(1s); + std::this_thread::sleep_for(1ms); } uv_loop_close(&m_uv_loop); @@ -220,13 +217,13 @@ auto client::run() -> void } m_is_running.exchange(true, std::memory_order_release); - int v = uv_run(&m_uv_loop, UV_RUN_DEFAULT); - std::cerr << "uv_run(UV_RUN_DEFAULT) = [" << v << "]\n"; - while (v > 0) + if (uv_run(&m_uv_loop, UV_RUN_DEFAULT) > 0) { - v = uv_run(&m_uv_loop, UV_RUN_NOWAIT); - std::cerr << "uv_run(UV_RUN_NOWAIT) = [" << v << "]\n"; - std::this_thread::sleep_for(std::chrono::milliseconds{1}); + // Run until all uv_handle_t objects are cleaned up. + while (uv_run(&m_uv_loop, UV_RUN_NOWAIT) > 0) + { + std::this_thread::sleep_for(1ms); + } } m_is_running.exchange(false, std::memory_order_release);