dnsdist: Simplify I/O handling for incoming H2 w/ nghttp2

diff --git a/pdns/dnsdist-doh-common.hh b/pdns/dnsdist-doh-common.hh
index f0a1adc76744649bf79807d5392f22e3cf36e07e..6e0cc86e03e9ecb6807bc297e2ca7d743af31188 100644 (file)
--- a/pdns/dnsdist-doh-common.hh
+++ b/pdns/dnsdist-doh-common.hh
@@ -77,7 +77,7 @@ struct DOHFrontend
   DOHFrontend()
   {
   }
-  DOHFrontend(std::shared_ptr<TLSCtx> tlsCtx):
+  DOHFrontend(std::shared_ptr<TLSCtx> tlsCtx) :
     d_tlsContext(std::move(tlsCtx))
   {
   }

diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
index b71601a916d4746198ca12f5036f96a75824bb4b..7945a2544f304493661092670ff5ceb9af487c97 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
+++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
@@ -375,11 +375,6 @@ void IncomingHTTP2Connection::handleIO()
       if (nghttp2_session_want_read(d_session.get()) != 0) {
         updateIO(IOState::NeedRead, handleReadableIOCallback);
       }
-      else {
-        if (isIdle()) {
-          watchForRemoteHostClosingConnection();
-        }
-      }
     }
   }
   catch (const std::exception& e) {
@@ -400,12 +395,7 @@ void IncomingHTTP2Connection::writeToSocket(bool socketReady)
       d_out.clear();
       d_outPos = 0;
       if (active() && !d_connectionClosing) {
-        if (!isIdle()) {
-          updateIO(IOState::NeedRead, handleReadableIOCallback);
-        }
-        else {
-          watchForRemoteHostClosingConnection();
-        }
+        updateIO(IOState::NeedRead, handleReadableIOCallback);
       }
       else {
         stopIO();
@@ -791,8 +781,7 @@ void IncomingHTTP2Connection::handleIncomingQuery(IncomingHTTP2Connection::Pendi
     sendResponse(streamID, query, code, d_ci.cs->dohFrontend->d_customResponseHeaders);
   };
 
-  if (query.d_method == PendingQuery::Method::Unknown ||
-      query.d_method == PendingQuery::Method::Unsupported) {
+  if (query.d_method == PendingQuery::Method::Unknown || query.d_method == PendingQuery::Method::Unsupported) {
     handleImmediateResponse(400, "DoH query not allowed because of unsupported HTTP method");
     return;
   }
@@ -919,10 +908,6 @@ int IncomingHTTP2Connection::on_frame_recv_callback(nghttp2_session* session, co
     auto stream = conn->d_currentStreams.find(streamID);
     if (stream != conn->d_currentStreams.end()) {
       conn->handleIncomingQuery(std::move(stream->second), streamID);
-
-      if (conn->isIdle()) {
-        conn->watchForRemoteHostClosingConnection();
-      }
     }
     else {
       vinfolog("Stream %d NOT FOUND", streamID);
@@ -954,10 +939,6 @@ int IncomingHTTP2Connection::on_stream_close_callback(nghttp2_session* session,
   auto request = std::move(stream->second);
   conn->d_currentStreams.erase(stream->first);
 
-  if (conn->isIdle()) {
-    conn->watchForRemoteHostClosingConnection();
-  }
-
   return 0;
 }
 
@@ -1222,20 +1203,6 @@ void IncomingHTTP2Connection::updateIO(IOState newState, const FDMultiplexer::ca
   }
 }
 
-void IncomingHTTP2Connection::watchForRemoteHostClosingConnection()
-{
-  if (d_connectionDied) {
-    return;
-  }
-
-  if (hasPendingWrite()) {
-    updateIO(IOState::NeedWrite, &handleWritableIOCallback);
-  }
-  else if (!d_connectionClosing) {
-    updateIO(IOState::NeedRead, handleReadableIOCallback);
-  }
-}
-
 void IncomingHTTP2Connection::handleIOError()
 {
   d_connectionDied = true;

diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.hh b/pdns/dnsdistdist/dnsdist-nghttp2-in.hh
index 3db7473a8e3d12a716b3db79109f568418c04d81..32af8d3087d4fffd940bc62a69485f3969191b0a 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-nghttp2-in.hh
+++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.hh
@@ -89,7 +89,6 @@ private:
   bool isIdle() const;
   uint32_t getConcurrentStreamsCount() const;
   void updateIO(IOState newState, const FDMultiplexer::callbackfunc_t& callback);
-  void watchForRemoteHostClosingConnection();
   void handleIOError();
   bool sendResponse(StreamID streamID, PendingQuery& context, uint16_t responseCode, const HeadersMap& customResponseHeaders, const std::string& contentType = "", bool addContentType = true);
   void handleIncomingQuery(PendingQuery&& query, StreamID streamID);

diff --git a/pdns/dnsdistdist/test-dnsdistnghttp2-in_cc.cc b/pdns/dnsdistdist/test-dnsdistnghttp2-in_cc.cc
index 0ac62b3c3b6d03945366877fc0cea8eb6a3e587b..48d2f6eeeb36d865d890e5dd33d2e644990f71c0 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistnghttp2-in_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistnghttp2-in_cc.cc
@@ -90,7 +90,7 @@ public:
   {
     const auto& context = s_connectionContexts.at(connectionID);
     d_clientOutBuffer.insert(d_clientOutBuffer.begin(), context.d_proxyProtocolPayload.begin(), context.d_proxyProtocolPayload.end());
-    
+
     nghttp2_session_callbacks* cbs = nullptr;
     nghttp2_session_callbacks_new(&cbs);
     std::unique_ptr<nghttp2_session_callbacks, void (*)(nghttp2_session_callbacks*)> callbacks(cbs, nghttp2_session_callbacks_del);
@@ -499,19 +499,19 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture)
     s_connectionContexts[counter++] = ExpectedData{{}, {query}, {response}, {403U}};
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* settings + headers + data client -> server.. */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128},
       /* .. continued */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60},
       /* headers + data */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max() },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max()},
       /* wait for next query, but the client closes the connection */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     auto state = std::make_shared<IncomingHTTP2Connection>(ConnectionInfo(&localCS, getBackendAddress("84", 4242)), threadData, now);
@@ -520,18 +520,18 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture)
 
   {
     /* client closes the connection right in the middle of sending the query */
-    s_connectionContexts[counter++] = ExpectedData{{}, {query}, {response}, { 403U }};
+    s_connectionContexts[counter++] = ExpectedData{{}, {query}, {response}, {403U}};
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* client sends one byte */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 1 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 1},
       /* then closes the connection */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     /* mark the incoming FD as always ready */
@@ -556,19 +556,19 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture)
 
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* settings + headers + data client -> server.. */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128},
       /* .. continued */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60},
       /* headers + data */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max() },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max()},
       /* wait for next query, but the client closes the connection */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     auto state = std::make_shared<IncomingHTTP2Connection>(ConnectionInfo(&localCS, getBackendAddress("84", 4242)), threadData, now);
@@ -587,17 +587,17 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture)
 
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* settings + headers + data client -> server.. */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128},
       /* .. continued */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60},
       /* we want to send the response but the client closes the connection */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     /* mark the incoming FD as always ready */
@@ -622,21 +622,21 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_SelfAnswered, TestFixture)
 
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* settings + headers + data client -> server.. */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128},
       /* .. continued */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60},
       /* headers + data (partial write) */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::NeedWrite, 1 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::NeedWrite, 1},
       /* nothing to read after that */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 0 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead, 0},
       /* then the client closes the connection before we are done  */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     /* mark the incoming FD as always ready */
@@ -691,25 +691,24 @@ BOOST_FIXTURE_TEST_CASE(test_IncomingConnection_BackendTimeout, TestFixture)
     s_connectionContexts[counter++] = ExpectedData{{}, {query}, {response}, {502U}};
     s_steps = {
       /* opening */
-      { ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::handshakeClient, IOState::Done},
       /* settings server -> client */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15 },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, 15},
       /* settings + headers + data client -> server.. */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 128},
       /* .. continued */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60 },
-        /* trying to read a new request while processing the first one */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 60},
+      /* trying to read a new request while processing the first one */
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::NeedRead},
       /* headers + data */
-      { ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max(), [&threadData](int desc) {
-          /* set the incoming descriptor as ready */
-          dynamic_cast<MockupFDMultiplexer*>(threadData.mplexer.get())->setReady(desc);
-        }
-      },
+      {ExpectedStep::ExpectedRequest::writeToClient, IOState::Done, std::numeric_limits<size_t>::max(), [&threadData](int desc) {
+         /* set the incoming descriptor as ready */
+         dynamic_cast<MockupFDMultiplexer*>(threadData.mplexer.get())->setReady(desc);
+       }},
       /* wait for next query, but the client closes the connection */
-      { ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0 },
+      {ExpectedStep::ExpectedRequest::readFromClient, IOState::Done, 0},
       /* server close */
-      { ExpectedStep::ExpectedRequest::closeClient, IOState::Done },
+      {ExpectedStep::ExpectedRequest::closeClient, IOState::Done},
     };
 
     auto state = std::make_shared<IncomingHTTP2Connection>(ConnectionInfo(&localCS, getBackendAddress("84", 4242)), threadData, now);