Skip to content
This repository

Fix bug that caused slow requests to block the processing of other concurrent requests #1

Closed
wants to merge 1 commit into from

2 participants

Damien Baty Chris McDonough
Damien Baty

As discussed in the mailing-list (http://groups.google.com/group/pylons-discuss/browse_thread/thread/1939cb1012e2190f), any request will block Waitress, which means that other concurrent requests will not be processed until the previous one has returned.

The attached changes provide a possible fix and a test. I do not know whether this change has negative side effects (but other tests still pass so we could pretend that nothing has been broken ;) ).

Chris McDonough
Owner

Ha, I just finished writing very similar code, checked in as 7fa8ef4 . Thanks for this though!

Chris McDonough
Owner

Fixed by commit #7fa8ef4d9410b83237dfd11f0700d3117eeb9b27

Chris McDonough mcdonc closed this February 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Feb 13, 2012
Fix bug that caused slow requests to block the processing of other co…
…ncurrent requests.
71c5aa9
This page is out of date. Refresh to see the latest.
6  waitress/channel.py
@@ -52,8 +52,6 @@ class HTTPChannel(logging_dispatcher, object):
52 52
     close_when_flushed = False   # set to True to close the socket when flushed
53 53
     requests = ()                # currently pending requests
54 54
     sent_continue = False        # used as a latch after sending 100 continue
55  
-    task_lock = thread.allocate_lock()  # lock used to push/pop requests
56  
-    outbuf_lock = thread.allocate_lock() # lock used to access any outbuf
57 55
     force_flush = False          # indicates a need to flush the outbuf
58 56
 
59 57
     #
@@ -74,6 +72,10 @@ def __init__(
74 72
         self.outbufs = [OverflowableBuffer(adj.outbuf_overflow)]
75 73
         self.creation_time = self.last_activity = time.time()
76 74
         asyncore.dispatcher.__init__(self, sock, map=map)
  75
+        # lock used to push/pop requests
  76
+        self.task_lock = thread.allocate_lock()
  77
+        # lock used to access any outbuf
  78
+        self.outbuf_lock = thread.allocate_lock()
77 79
 
78 80
     def any_outbuf_has_data(self):
79 81
         for outbuf in self.outbufs:
24  waitress/tests/fixtureapps/slow.py
... ...
@@ -0,0 +1,24 @@
  1
+import time
  2
+
  3
+def app(environ, start_response):
  4
+    path_info = environ['PATH_INFO']
  5
+    if path_info == '/slow':
  6
+        time.sleep(1)
  7
+        body = b'slow'
  8
+    else:
  9
+        body = b'quick'
  10
+    cl = str(len(body))
  11
+    start_response(
  12
+        '200 OK',
  13
+        [('Content-Length', cl), ('Content-Type', 'text/plain')])
  14
+    return [body]
  15
+
  16
+if __name__ == '__main__':
  17
+    import logging
  18
+    class NullHandler(logging.Handler):
  19
+        def emit(self, record):
  20
+            pass
  21
+    h = NullHandler()
  22
+    logging.getLogger('waitress').addHandler(h)
  23
+    from waitress import serve
  24
+    serve(app, port=61523, _quiet=True, expose_tracebacks=True)
38  waitress/tests/test_functional.py
@@ -1153,6 +1153,44 @@ def test_notfilelike_nocl_http10(self):
1153 1153
         self.sock.send(to_send)
1154 1154
         self.assertRaises(ConnectionClosed, read_http, fp)
1155 1155
 
  1156
+
  1157
+class TestConcurrentRequests(SubprocessTests, unittest.TestCase):
  1158
+
  1159
+    def setUp(self):
  1160
+        slow = os.path.join(here, 'fixtureapps', 'slow.py')
  1161
+        self.start_subprocess([self.exe, slow])
  1162
+        self.bodies = []
  1163
+        self.sent = []
  1164
+
  1165
+    def tearDown(self):
  1166
+        self.stop_subprocess()
  1167
+
  1168
+    def _make_socket(self):
  1169
+        return socket.socket(socket.AF_INET, socket.SOCK_STREAM)
  1170
+
  1171
+    def _send_request(self, path):
  1172
+        to_send = "GET %s HTTP/1.0\n\n" % path
  1173
+        to_send = tobytes(to_send)
  1174
+        socket = self._make_socket()
  1175
+        socket.connect((self.host, self.port))
  1176
+        socket.send(to_send)
  1177
+        self.sent.append(path)
  1178
+        fp = socket.makefile('rb', 0)
  1179
+        line, headers, response_body = read_http(fp)
  1180
+        self.bodies.append(response_body)
  1181
+
  1182
+    def test_slow_requests_do_not_block_other_threads(self):
  1183
+        import thread
  1184
+        thread.start_new_thread(self._send_request, ('/slow', ))
  1185
+        # wait a bit to make sure that '/slow' is requested before '/quick'
  1186
+        time.sleep(0.3)
  1187
+        thread.start_new_thread(self._send_request, ('/quick', ))
  1188
+        while len(self.bodies) < 2:
  1189
+            time.sleep(.1)  # wait for requests to be sent and processed
  1190
+        self.assertEqual(self.sent, ['/slow', '/quick'])
  1191
+        self.assertEqual(self.bodies, ['quick', 'slow'])
  1192
+
  1193
+
1156 1194
 def parse_headers(fp):
1157 1195
     """Parses only RFC2822 headers from a file pointer.
1158 1196
     """
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.