From 62af150ded89da4ae922e0ad6dc82af9868e7a5e Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 16 Sep 2025 17:24:50 +0200 Subject: [PATCH] dnsdist: Fix reentry issue in TCP downstream I/O on macOS/BSD Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-nghttp2.cc | 1 - pdns/dnsdistdist/dnsdist-tcp-downstream.cc | 37 ++++++++++++++++------ pdns/dnsdistdist/dnsdist-tcp-downstream.hh | 2 ++ pdns/dnsdistdist/dnsdist-tcp.cc | 1 - pdns/dnsdistdist/dnsdist-tcp.hh | 1 + pdns/dnsdistdist/test-dnsdisttcp_cc.cc | 20 ++++++++---- 6 files changed, 43 insertions(+), 19 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.cc b/pdns/dnsdistdist/dnsdist-nghttp2.cc index 1e289901f..c5be3a1c8 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2.cc @@ -379,7 +379,6 @@ void DoHConnectionToBackend::handleReadableIOCallback(int fd, FDMultiplexer::fun if (conn->d_inIOCallback) { return; } - conn->d_inIOCallback = true; dnsdist::tcp::HandlingIOGuard handlingIOGuard(conn->d_inIOCallback); IOStateGuard ioGuard(conn->d_ioState); do { diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc index 1c91bba13..0e126bcf8 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc +++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc @@ -279,6 +279,7 @@ IOState TCPConnectionToBackend::sendQuery(std::shared_ptrd_handler->tryWrite(conn->d_currentQuery.d_query.d_buffer, conn->d_currentPos, conn->d_currentQuery.d_query.d_buffer.size()); if (state != IOState::Done) { + conn->d_lastIOBlocked = true; return state; } @@ -310,6 +311,12 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c throw std::runtime_error("No downstream socket in " + std::string(__PRETTY_FUNCTION__) + "!"); } + if (conn->d_handlingIO) { + return; + } + conn->d_handlingIO = true; + dnsdist::tcp::HandlingIOGuard reentryGuard(conn->d_handlingIO); + bool connectionDied = false; IOState iostate = IOState::Done; IOStateGuard ioGuard(conn->d_ioState); @@ -317,6 +324,7 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c do { reconnected = false; + conn->d_lastIOBlocked = false; try { if (conn->d_state == State::sendingQueryToBackend) { @@ -352,8 +360,11 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c conn->d_currentPos = 0; conn->d_lastDataReceivedTime = now; } - else if (conn->d_state == State::waitingForResponseFromBackend && conn->d_currentPos > 0) { - conn->d_state = State::readingResponseSizeFromBackend; + else { + conn->d_lastIOBlocked = true; + if (conn->d_state == State::waitingForResponseFromBackend && conn->d_currentPos > 0) { + conn->d_state = State::readingResponseSizeFromBackend; + } } } @@ -373,6 +384,9 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c return; } } + else { + conn->d_lastIOBlocked = true; + } } if (conn->d_state != State::idle && @@ -506,7 +520,7 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c } } } - while (reconnected); + while (reconnected || (iostate != IOState::Done && !conn->d_connectionDied && !conn->d_lastIOBlocked)); ioGuard.release(); } @@ -760,16 +774,19 @@ IOState TCPConnectionToBackend::handleResponse(std::shared_ptr 10k self-generated pipelined on the same connection"); /* 10k self-generated REFUSED pipelined on the same connection */ @@ -699,7 +698,6 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture) auto state = std::make_shared(ConnectionInfo(&localCS, getBackendAddress("84", 4242)), threadData, now); state->handleIO(); BOOST_CHECK_EQUAL(s_writeBuffer.size(), query.size() * count); -#endif } { @@ -1819,7 +1817,6 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_BackendNoOOOR, TestFixture) } { -#if 1 /* 101 queries on the same connection, check that the maximum number of queries kicks in */ TEST_INIT("=> 101 queries on the same connection"); @@ -1886,7 +1883,6 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_BackendNoOOOR, TestFixture) dnsdist::configuration::updateRuntimeConfiguration([](dnsdist::configuration::RuntimeConfiguration& config) { config.d_maxTCPQueriesPerConn = 0; }); -#endif } { @@ -2206,8 +2202,6 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) /* set the client descriptor as NOT ready */ dynamic_cast(threadData.mplexer.get())->setNotReady(desc); } }, - /* reading from the client (not ready) */ - { ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 0 }, /* reading a response from the backend (5) */ { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(4).size() - 2 }, { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(4).size() }, @@ -2220,6 +2214,8 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) dynamic_cast(threadData.mplexer.get())->setNotReady(desc); timeout = true; } }, + /* reading from the client (not ready) */ + { ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 0 }, /* A timeout occurs */ @@ -3964,10 +3960,14 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, 2 }, { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(0).size() - 2 }, /* sending it to the client */ - { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, responses.at(0).size(), [&threadData](int desc) { + { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, responses.at(0).size(), [&threadData,&backend1Desc](int desc) { /* client becomes readable */ dynamic_cast(threadData.mplexer.get())->setReady(desc); + /* first backend is no longer readable */ + dynamic_cast(threadData.mplexer.get())->setNotReady(backend1Desc); } }, + /* no response ready from the backend yet */ + { ExpectedStep::ExpectedRequest::readFromBackend, IOState::NeedRead, 0 }, /* reading a query from the client (5) */ { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 2 }, { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, queries.at(4).size() - 2, [&threadData](int desc) { @@ -3993,6 +3993,8 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) dynamic_cast(threadData.mplexer.get())->setNotReady(backend1Desc); dynamic_cast(threadData.mplexer.get())->setReady(backend2Desc); } }, + /* no more response ready yet from backend 1 */ + { ExpectedStep::ExpectedRequest::readFromBackend, IOState::NeedRead, 0 }, /* reading response (3) from the second backend (2) */ { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, 2 }, { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(2).size() - 2 }, @@ -4003,6 +4005,8 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) dynamic_cast(threadData.mplexer.get())->setNotReady(backend2Desc); dynamic_cast(threadData.mplexer.get())->setReady(backend1Desc); } }, + /* no more response ready yet from backend 2 */ + { ExpectedStep::ExpectedRequest::readFromBackend, IOState::NeedRead, 0 }, /* reading response (5) from the first backend (1) */ { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, 2 }, { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(4).size() - 2 }, @@ -4013,6 +4017,8 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnectionOOOR_BackendOOOR, TestFixture) dynamic_cast(threadData.mplexer.get())->setNotReady(backend1Desc); dynamic_cast(threadData.mplexer.get())->setReady(backend2Desc); } }, + /* no more response ready yet from backend 1 */ + { ExpectedStep::ExpectedRequest::readFromBackend, IOState::NeedRead, 0 }, /* reading response (4) from the second backend (2) */ { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, 2 }, { ExpectedStep::ExpectedRequest::readFromBackend, IOState::Done, responses.at(3).size() - 2 }, -- 2.47.3