Stop reading a connection a worker has taken over

4 files changed, 31 insertions, 0 deletions

diff --git a/src/ot_http.c b/src/ot_http.c
index e6a8db7..29f41a3 100644
--- a/src/ot_http.c
+++ b/src/ot_http.c
@@ -367,6 +367,7 @@ static ssize_t http_handle_stats(const int64_t sock, struct ot_workstruct *ws, c
        a home of its own and only then be made reachable by a worker thread */
     if (!http_adopt(sock))
       HTTPERROR_500;
+    ws->handed_off = 1;
     fullscrape_deliver(sock, format);
     return ws->reply_size = -2;
   }
@@ -383,6 +384,7 @@ static ssize_t http_handle_stats(const int64_t sock, struct ot_workstruct *ws, c
 
     if (!http_adopt(sock))
       HTTPERROR_500;
+    ws->handed_off = 1;
     stats_deliver(sock, mode);
     return ws->reply_size = -2;
   }
@@ -477,6 +479,7 @@ static ssize_t http_handle_fullscrape(const int64_t sock, struct ot_workstruct *
 
   if (!http_adopt(sock))
     HTTPERROR_500;
+  ws->handed_off = 1;
   fullscrape_deliver(sock, format);
   return ws->reply_size = -2;
 }
@@ -742,6 +745,7 @@ ssize_t http_handle_request(const int64_t sock, struct ot_workstruct *ws) {
      subroutine gets to answer: an error reply finishes with the connection
      on its way out, and ws outlives the connection it was serving */
   ws->keep_alive = 0;
+  ws->handed_off = 0;
 #ifdef WANT_KEEPALIVE
   if ((write_ptr = http_header(ws->request, ws->header_size, "connection")) && (*write_ptr == 'K' || *write_ptr == 'k'))
     ws->keep_alive = 1;
diff --git a/src/ot_tcp.c b/src/ot_tcp.c
index 1351b92..f93795b 100644
--- a/src/ot_tcp.c
+++ b/src/ot_tcp.c
@@ -79,6 +79,11 @@ void tcp_read(const int64_t sock, struct ot_workstruct *ws) {
     ws->request      = cookie->request.data;
     ws->request_size = (ssize_t)cookie->request.size;
     http_handle_request(sock, ws);
+
+    /* A worker owns the connection now, and the buffer this loop reads hangs
+       off it, so there is nothing here left to look at */
+    if (ws->handed_off)
+      return;
 #ifdef WANT_KEEPALIVE
     if (!ws->keep_alive)
 #endif
diff --git a/src/trackerlogic.h b/src/trackerlogic.h
index 4f970ba..d03d3b1 100644
--- a/src/trackerlogic.h
+++ b/src/trackerlogic.h
@@ -256,6 +256,9 @@ struct ot_workstruct {
   uint16_t rand48_state[3];
 
   int      keep_alive;
+
+  /* Set when a request path gave the connection to a worker thread. */
+  int      handed_off;
 };
 
 /* Everything a thread that serves requests needs for itself. Set up and torn
diff --git a/tests/integration/test_keepalive.py b/tests/integration/test_keepalive.py
index 01f02a5..2c724fc 100644
--- a/tests/integration/test_keepalive.py
+++ b/tests/integration/test_keepalive.py
@@ -7,6 +7,7 @@ the request has moved on, so whether to keep the connection has to be
 remembered on the connection itself.
 """
 
+import time
 import unittest
 import urllib.parse
 
@@ -106,6 +107,24 @@ class KeepAlive(TrackerTestCase):
         finally:
             c.close()
 
+    def test_a_split_request_answered_by_a_worker_does_not_spin(self):
+        """A reply that a worker produces does not go through the send path,
+        so the request stays in the buffer the read loop is reading. The loop
+        used to find the same request again on every pass, queue a task for
+        it, and never hand the thread back to the event loop."""
+        for path in ("/stats", "/scrape"):
+            c = self.tracker.connect()
+            try:
+                c.sock.sendall(("GET %s HTTP/1.1\r\nConnection: keep-alive\r\n" % path).encode())
+                time.sleep(0.2)
+                c.sock.sendall(b"\r\n")
+                self.assertIn(b"200", c.reply()[0], "%s was not answered" % path)
+            finally:
+                c.close()
+
+            answer = self.tracker.announce(self.hash_for("after-" + path), port=6881, left=0)
+            self.assertNotIn(b"failure reason", answer, "the tracker stopped serving after %s" % path)
+
 
 if __name__ == "__main__":
     unittest.main()