From 7dfd36ce539de4b22427d2b7c3e2bf00146861cd Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 18 May 2026 12:34:21 +0200 Subject: [PATCH 1/5] dnsdist: Handle exceptions when dealing with asynchronous objects Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-async.cc | 98 +++++++++++++++++-------------- 1 file changed, 54 insertions(+), 44 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-async.cc b/pdns/dnsdistdist/dnsdist-async.cc index 8bd0caa46e66..0d0ba896e441 100644 --- a/pdns/dnsdistdist/dnsdist-async.cc +++ b/pdns/dnsdistdist/dnsdist-async.cc @@ -94,60 +94,70 @@ void AsynchronousHolder::mainThread(std::shared_ptr data) std::vector readyFDs; while (true) { - bool shouldWait = true; - int timeout = -1; - dnsdist::configuration::refreshLocalRuntimeConfiguration(); - - { - auto content = data->d_content.lock(); - if (data->d_done) { - return; - } + try { + bool shouldWait = true; + int timeout = -1; + dnsdist::configuration::refreshLocalRuntimeConfiguration(); - if (!content->empty()) { - gettimeofday(&now, nullptr); - struct timeval next = getNextTTD(*content); - if (next <= now) { - pickupExpired(*content, now, expiredEvents); - shouldWait = false; + { + auto content = data->d_content.lock(); + if (data->d_done) { + return; } - else { - auto remainingUsec = uSec(next - now); - timeout = static_cast(std::round(static_cast(remainingUsec) / 1000.0)); - if (timeout == 0 && remainingUsec > 0) { - /* if we have less than 1 ms, let's wait at least 1 ms */ - timeout = 1; + + if (!content->empty()) { + gettimeofday(&now, nullptr); + struct timeval next = getNextTTD(*content); + if (next <= now) { + pickupExpired(*content, now, expiredEvents); + shouldWait = false; + } + else { + auto remainingUsec = uSec(next - now); + timeout = static_cast(std::round(static_cast(remainingUsec) / 1000.0)); + if (timeout == 0 && remainingUsec > 0) { + /* if we have less than 1 ms, let's wait at least 1 ms */ + timeout = 1; + } } } } - } - if (shouldWait) { - auto timedOut = wait(*data, *mplexer, readyFDs, timeout); - if (timedOut) { - auto content = data->d_content.lock(); - gettimeofday(&now, nullptr); - pickupExpired(*content, now, expiredEvents); + if (shouldWait) { + auto timedOut = wait(*data, *mplexer, readyFDs, timeout); + if (timedOut) { + auto content = data->d_content.lock(); + gettimeofday(&now, nullptr); + pickupExpired(*content, now, expiredEvents); + } } - } - while (!expiredEvents.empty()) { - auto [queryID, query] = std::move(expiredEvents.front()); - expiredEvents.pop_front(); - if (!data->d_failOpen) { - VERBOSESLOG(infolog("Asynchronous query %d has expired at %d.%d, notifying the sender", queryID, now.tv_sec, now.tv_usec), - dnsdist::logging::getTopLogger("async-thread")->info(Logr::Info, "Asynchronous query has expired, notifying the sender", "dns.question.id", Logging::Loggable(queryID))); - auto sender = query->getTCPQuerySender(); - if (sender) { - TCPResponse tresponse(std::move(query->query)); - sender->notifyIOError(now, std::move(tresponse)); + while (!expiredEvents.empty()) { + auto [queryID, query] = std::move(expiredEvents.front()); + expiredEvents.pop_front(); + if (!data->d_failOpen) { + VERBOSESLOG(infolog("Asynchronous query %d has expired at %d.%d, notifying the sender", queryID, now.tv_sec, now.tv_usec), + dnsdist::logging::getTopLogger("async-thread")->info(Logr::Info, "Asynchronous query has expired, notifying the sender", "dns.question.id", Logging::Loggable(queryID))); + auto sender = query->getTCPQuerySender(); + if (sender) { + TCPResponse tresponse(std::move(query->query)); + sender->notifyIOError(now, std::move(tresponse)); + } + } + else { + VERBOSESLOG(infolog("Asynchronous query %d has expired at %d.%d, resuming", queryID, now.tv_sec, now.tv_usec), + dnsdist::logging::getTopLogger("async-thread")->info(Logr::Info, "Asynchronous query has expired, resuming", "dns.question.id", Logging::Loggable(queryID))); + resumeQuery(std::move(query)); } } - else { - VERBOSESLOG(infolog("Asynchronous query %d has expired at %d.%d, resuming", queryID, now.tv_sec, now.tv_usec), - dnsdist::logging::getTopLogger("async-thread")->info(Logr::Info, "Asynchronous query has expired, resuming", "dns.question.id", Logging::Loggable(queryID))); - resumeQuery(std::move(query)); - } + } + catch (const std::exception& exp) { + VERBOSESLOG(infolog("Got exception in the main asynchronous handler thread: %s", exp.what()), + dnsdist::logging::getTopLogger("async-thread")->error(Logr::Info, exp.what(), "Got exception in the main asynchronous handler thread")); + } + catch (...) { + VERBOSESLOG(infolog("Got exception in the main asynchronous handler thread"), + dnsdist::logging::getTopLogger("async-thread")->info(Logr::Info, "Got exception in the main asynchronous handler thread")); } } } From 4b205eaee081af5448ca7d181e8132dc40165b7a Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 18 May 2026 12:36:04 +0200 Subject: [PATCH 2/5] dnsdist: Better handling of exceptions in outgoing DoH We cannot throw exceptions over the C++/C boundary. Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-nghttp2.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.cc b/pdns/dnsdistdist/dnsdist-nghttp2.cc index 37ce3cd242cb..e9f1e402e830 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2.cc @@ -309,7 +309,9 @@ void DoHConnectionToBackend::queueQuery(std::shared_ptr& sender, PendingRequest pending; pending.d_query = std::move(query); - pending.d_sender = std::move(sender); + /* don't move the sender, we don't own it at this point and the caller might need it, + especially if we throw below */ + pending.d_sender = sender; uint32_t tentativeStreamId = nghttp2_session_get_next_stream_id(d_session.get()); if (tentativeStreamId == static_cast(1 << 31)) { @@ -714,7 +716,7 @@ int DoHConnectionToBackend::on_data_chunk_recv_callback(nghttp2_session* session int DoHConnectionToBackend::on_stream_close_callback(nghttp2_session* session, StreamID stream_id, uint32_t error_code, void* user_data) { (void)session; - DoHConnectionToBackend* conn = reinterpret_cast(user_data); + auto* conn = reinterpret_cast(user_data); if (error_code == 0) { return 0; @@ -738,8 +740,16 @@ int DoHConnectionToBackend::on_stream_close_callback(nghttp2_session* session, S if (request.d_query.d_downstreamFailures < conn->d_ds->d_config.d_retries) { ++request.d_query.d_downstreamFailures; - auto downstream = t_downstreamDoHConnectionsManager.getConnectionToDownstream(conn->d_mplexer, conn->d_ds, now, std::string(conn->d_proxyProtocolPayload)); - downstream->queueQuery(request.d_sender, std::move(request.d_query)); + try { + auto downstream = t_downstreamDoHConnectionsManager.getConnectionToDownstream(conn->d_mplexer, conn->d_ds, now, std::string(conn->d_proxyProtocolPayload)); + downstream->queueQuery(request.d_sender, std::move(request.d_query)); + } + catch (const std::exception& exp) { + ++conn->d_ds->tcpDiedSendingQuery; + VERBOSESLOG(infolog("Failed to retry DoH query after stream close: %s", exp.what()), + conn->getLogger()->error(Logr::Info, exp.what(), "Failed to retry DoH query after stream close")); + conn->handleResponseError(std::move(request), now); + } } else { conn->handleResponseError(std::move(request), now); From 14c530145fab2cee945620629870226a2204fa97 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 18 May 2026 12:37:30 +0200 Subject: [PATCH 3/5] tcpiohandler: Don't throw exceptions over the C/C++ boundary Signed-off-by: Remi Gacogne --- pdns/tcpiohandler.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc index 7ee5cfbaa05a..176b20c64b48 100644 --- a/pdns/tcpiohandler.cc +++ b/pdns/tcpiohandler.cc @@ -936,7 +936,13 @@ class OpenSSLTLSIOCtx: public TLSCtx, public std::enable_shared_from_thisaddNewTicket(session); + try { + conn->addNewTicket(session); + } + // NOLINTNEXTLINE(bugprone-empty-catch) + catch (...) { + } + return 1; } @@ -1299,7 +1305,7 @@ class GnuTLSConnection: public TLSConnection return 0; } - GnuTLSConnection* conn = reinterpret_cast(gnutls_session_get_ptr(session)); + auto* conn = reinterpret_cast(gnutls_session_get_ptr(session)); if (conn == nullptr) { return 0; } @@ -1308,7 +1314,10 @@ class GnuTLSConnection: public TLSConnection auto ret = gnutls_session_get_data2(session, &sess); /* GnuTLS returns a 'fake' ticket of 4 bytes set to zero when there is no ticket available */ if (ret != GNUTLS_E_SUCCESS || sess.size <= 4) { - throw std::runtime_error("Error getting GnuTLSSession: " + std::string(gnutls_strerror(ret))); + if (sess.data != nullptr) { + gnutls_free(sess.data); + } + return 0; } conn->d_tlsSessions.push_back(std::make_unique(sess)); return 0; From 2c248a60d104891042283fdcc3386ed356b29e5b Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 18 May 2026 14:50:22 +0200 Subject: [PATCH 4/5] tcpiohandler: Appease clang-tidy Signed-off-by: Remi Gacogne --- pdns/tcpiohandler.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc index 176b20c64b48..be87ebbfdf71 100644 --- a/pdns/tcpiohandler.cc +++ b/pdns/tcpiohandler.cc @@ -1305,6 +1305,7 @@ class GnuTLSConnection: public TLSConnection return 0; } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): GnuTLS API auto* conn = reinterpret_cast(gnutls_session_get_ptr(session)); if (conn == nullptr) { return 0; From 0538445122506972454cd9272a2ced4e057f581f Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 18 May 2026 16:10:43 +0200 Subject: [PATCH 5/5] dnsdist: Silence clang-tidy Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-nghttp2.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.cc b/pdns/dnsdistdist/dnsdist-nghttp2.cc index e9f1e402e830..115fd10e6105 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2.cc @@ -716,6 +716,7 @@ int DoHConnectionToBackend::on_data_chunk_recv_callback(nghttp2_session* session int DoHConnectionToBackend::on_stream_close_callback(nghttp2_session* session, StreamID stream_id, uint32_t error_code, void* user_data) { (void)session; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): nghttp2 API auto* conn = reinterpret_cast(user_data); if (error_code == 0) {