From 4a671d717a7bb4ae4c7da0a70f1ed351da67280d Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 6 May 2024 16:36:39 +0200 Subject: [PATCH] dnsdist: Implement an "atExit" Lua callback to clean up leftovers --- pdns/dnsdistdist/dnsdist-lsan.supp | 4 ++-- pdns/dnsdistdist/dnsdist-lua-network.cc | 5 +++++ pdns/dnsdistdist/dnsdist.cc | 11 ++++++++--- pdns/dnsdistdist/doh.cc | 12 +++++++----- regression-tests.dnsdist/test_Async.py | 10 ++++++++++ regression-tests.dnsdist/test_Routing.py | 5 +++++ 6 files changed, 37 insertions(+), 10 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lsan.supp b/pdns/dnsdistdist/dnsdist-lsan.supp index a04f7122a8..64bf7122c5 100644 --- a/pdns/dnsdistdist/dnsdist-lsan.supp +++ b/pdns/dnsdistdist/dnsdist-lsan.supp @@ -1,2 +1,2 @@ -# h2o -leak:create_socket +# it's hard to clean up the per-thread Lua load-balancing policies on exit +leak:setupLuaLoadBalancingContext diff --git a/pdns/dnsdistdist/dnsdist-lua-network.cc b/pdns/dnsdistdist/dnsdist-lua-network.cc index 56a58cd5cf..58ad214ad5 100644 --- a/pdns/dnsdistdist/dnsdist-lua-network.cc +++ b/pdns/dnsdistdist/dnsdist-lua-network.cc @@ -41,6 +41,11 @@ NetworkListener::NetworkListener() : NetworkListener::~NetworkListener() { d_data->d_exiting = true; + + /* wake up the listening thread */ + for (const auto& socket : d_data->d_sockets) { + shutdown(socket.second.getHandle(), SHUT_RD); + } } void NetworkListener::readCB(int desc, FDMultiplexer::funcparam_t& param) diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index 1911e34180..954450202f 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -2810,9 +2810,14 @@ static void sigTermHandler([[maybe_unused]] int sig) std::cout << "Exiting on user request" << std::endl; #endif /* __SANITIZE_THREAD__ */ #if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE) - auto lock = g_lua.lock(); - cleanupLuaObjects(); - *lock = LuaContext(); + if (dnsdist::g_asyncHolder) { + dnsdist::g_asyncHolder->stop(); + } + { + auto lock = g_lua.lock(); + cleanupLuaObjects(); + *lock = LuaContext(); + } __lsan_do_leak_check(); #endif /* __SANITIZE_ADDRESS__ && HAVE_LEAK_SANITIZER_INTERFACE */ _exit(EXIT_SUCCESS); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 292e0d68ca..6bdd576471 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -212,6 +212,7 @@ struct DOHServerConfig std::set> paths; h2o_globalconf_t h2o_config{}; h2o_context_t h2o_ctx{}; + std::unique_ptr h2o_socket{nullptr, h2o_socket_close}; std::shared_ptr accept_ctx{nullptr}; ClientState* clientState{nullptr}; std::shared_ptr dohFrontend{nullptr}; @@ -1430,9 +1431,9 @@ static void on_accept(h2o_socket_t *listener, const char *err) static int create_listener(std::shared_ptr& dsc, int descriptor) { - auto* sock = h2o_evloop_socket_create(dsc->h2o_ctx.loop, descriptor, H2O_SOCKET_FLAG_DONT_READ); - sock->data = dsc.get(); - h2o_socket_read_start(sock, on_accept); + dsc->h2o_socket = std::unique_ptr{h2o_evloop_socket_create(dsc->h2o_ctx.loop, descriptor, H2O_SOCKET_FLAG_DONT_READ), &h2o_socket_close}; + dsc->h2o_socket->data = dsc.get(); + h2o_socket_read_start(dsc->h2o_socket.get(), on_accept); return 0; } @@ -1599,11 +1600,11 @@ void dohThread(ClientState* clientState) dsc->h2o_ctx.storage.entries[0].data = dsc.get(); ++dsc->h2o_ctx.storage.size; - auto* sock = h2o_evloop_socket_create(dsc->h2o_ctx.loop, dsc->d_responseReceiver.getDescriptor(), H2O_SOCKET_FLAG_DONT_READ); + auto sock = std::unique_ptr{h2o_evloop_socket_create(dsc->h2o_ctx.loop, dsc->d_responseReceiver.getDescriptor(), H2O_SOCKET_FLAG_DONT_READ), &h2o_socket_close}; sock->data = dsc.get(); // this listens to responses from dnsdist to turn into http responses - h2o_socket_read_start(sock, on_dnsdist); + h2o_socket_read_start(sock.get(), on_dnsdist); setupAcceptContext(*dsc->accept_ctx, *dsc, false); @@ -1628,6 +1629,7 @@ void dohThread(ClientState* clientState) } while (!stop); + h2o_evloop_destroy(dsc->h2o_ctx.loop); } catch (const std::exception& e) { throw runtime_error("DOH thread failed to launch: " + std::string(e.what())); diff --git a/regression-tests.dnsdist/test_Async.py b/regression-tests.dnsdist/test_Async.py index eda4a5903c..bb37c7ee12 100644 --- a/regression-tests.dnsdist/test_Async.py +++ b/regression-tests.dnsdist/test_Async.py @@ -510,6 +510,11 @@ class TestAsyncFFI(DNSDistTest, AsyncTests): return DNSResponseAction.Allow end + function atExit() + listener = nil + collectgarbage() + end + -- this only matters for tests actually reaching the backend addAction('tcp-only.async.tests.powerdns.com', PoolAction('tcp-only', false)) addAction('cache.async.tests.powerdns.com', PoolAction('cache', false)) @@ -618,6 +623,11 @@ class TestAsyncLua(DNSDistTest, AsyncTests): return DNSResponseAction.Allow end + function atExit() + listener = nil + collectgarbage() + end + -- this only matters for tests actually reaching the backend addAction('tcp-only.async.tests.powerdns.com', PoolAction('tcp-only', false)) addAction('cache.async.tests.powerdns.com', PoolAction('cache', false)) diff --git a/regression-tests.dnsdist/test_Routing.py b/regression-tests.dnsdist/test_Routing.py index 186cba0692..1f32f98d73 100644 --- a/regression-tests.dnsdist/test_Routing.py +++ b/regression-tests.dnsdist/test_Routing.py @@ -352,6 +352,11 @@ class TestRoutingLuaFFIPerThreadRoundRobinLB(RoundRobinTest, DNSDistTest): s1:setUp() s2 = newServer{address="127.0.0.1:%s"} s2:setUp() + + function atExit() + setServerPolicy(leastOutstanding) + collectgarbage() + end """ @classmethod -- 2.47.2