optimize tracking of pending outbuf bytesfix-deadlock-on-disconnect

2 files changed, 23 insertions, 19 deletions

diff --git a/waitress/channel.py b/waitress/channel.py
index cb829f0..fd5a3f5 100644
--- a/waitress/channel.py
+++ b/waitress/channel.py
@@ -70,6 +70,7 @@ class HTTPChannel(wasyncore.dispatcher, object):
         self.server = server
         self.adj = adj
         self.outbufs = [OverflowableBuffer(adj.outbuf_overflow)]
+        self.total_outbufs_len = 0
         self.creation_time = self.last_activity = time.time()
 
         # task_lock used to push/pop requests
@@ -83,16 +84,7 @@ class HTTPChannel(wasyncore.dispatcher, object):
         self.addr = addr
 
     def any_outbuf_has_data(self):
-        for outbuf in self.outbufs:
-            if bool(outbuf):
-                return True
-        return False
-
-    def total_outbufs_len(self):
-        # genexpr == more funccalls
-        # use b.__len__ rather than len(b) FBO of not getting OverflowError
-        # on Python 2
-        return sum([b.__len__() for b in self.outbufs]) 
+        return self.total_outbufs_len > 0
 
     def writable(self):
         # if there's data in the out buffer or we've been instructed to close
@@ -124,7 +116,7 @@ class HTTPChannel(wasyncore.dispatcher, object):
             #    won't get done.
             flush = self._flush_some_if_lockable
             self.force_flush = False
-        elif (self.total_outbufs_len() >= self.adj.send_bytes):
+        elif (self.total_outbufs_len >= self.adj.send_bytes):
             # 1. There's a running task, so we need to try to lock
             #    the outbuf before sending
             # 2. Only try to send if the data in the out buffer is larger
@@ -196,7 +188,9 @@ class HTTPChannel(wasyncore.dispatcher, object):
                 if not self.sent_continue:
                     # there's no current task, so we don't need to try to
                     # lock the outbuf to append to it.
-                    self.outbufs[-1].append(b'HTTP/1.1 100 Continue\r\n\r\n')
+                    outbuf_payload = b'HTTP/1.1 100 Continue\r\n\r\n'
+                    self.outbufs[-1].append(outbuf_payload)
+                    self.total_outbufs_len += len(outbuf_payload)
                     self.sent_continue = True
                     self._flush_some()
                     request.completed = False
@@ -261,6 +255,7 @@ class HTTPChannel(wasyncore.dispatcher, object):
                     outbuf.skip(num_sent, True)
                     outbuflen -= num_sent
                     sent += num_sent
+                    self.total_outbufs_len -= num_sent
                 else:
                     dobreak = True
                     break
@@ -283,6 +278,7 @@ class HTTPChannel(wasyncore.dispatcher, object):
                 except:
                     self.logger.exception(
                         'Unknown exception while trying to close outbuf')
+            self.total_outbufs_len = 0
             self.connected = False
         wasyncore.dispatcher.close(self)
 
@@ -329,11 +325,13 @@ class HTTPChannel(wasyncore.dispatcher, object):
                     self.outbufs.append(nextbuf)
                 else:
                     self.outbufs[-1].append(data)
+                num_bytes = len(data)
+                self.total_outbufs_len += num_bytes
             # XXX We might eventually need to pull the trigger here (to
             # instruct select to stop blocking), but it slows things down so
             # much that I'll hold off for now; "server push" on otherwise
             # unbusy systems may suffer.
-            return len(data)
+            return num_bytes
         return 0
 
     def service(self):
diff --git a/waitress/tests/test_channel.py b/waitress/tests/test_channel.py
index 7efd3b3..df20dab 100644
--- a/waitress/tests/test_channel.py
+++ b/waitress/tests/test_channel.py
@@ -25,18 +25,23 @@ class TestHTTPChannel(unittest.TestCase):
     def test_total_outbufs_len_an_outbuf_size_gt_sys_maxint(self):
         from waitress.compat import MAXINT
         inst, _, map = self._makeOneWithMap()
-        class DummyHugeBuffer(object):
+        class DummyBuffer(object):
+            chunks = []
+            def append(self, data):
+                self.chunks.append(data)
+        class DummyData(object):
             def __len__(self):
-                return MAXINT + 1
-        inst.outbufs = [DummyHugeBuffer()]
-        result = inst.total_outbufs_len()
+                return MAXINT
+        inst.total_outbufs_len = 1
+        inst.outbufs = [DummyBuffer()]
+        inst.write_soon(DummyData())
         # we are testing that this method does not raise an OverflowError
         # (see https://github.com/Pylons/waitress/issues/47)
-        self.assertEqual(result, MAXINT+1)
+        self.assertEqual(inst.total_outbufs_len, MAXINT+1)
 
     def test_writable_something_in_outbuf(self):
         inst, sock, map = self._makeOneWithMap()
-        inst.outbufs[0].append(b'abc')
+        inst.total_outbufs_len = 3
         self.assertTrue(inst.writable())
 
     def test_writable_nothing_in_outbuf(self):
@@ -132,6 +137,7 @@ class TestHTTPChannel(unittest.TestCase):
         inst, sock, map = self._makeOneWithMap()
         inst.requests = [True]
         inst.outbufs = [DummyBuffer(b'abc')]
+        inst.total_outbufs_len = 3
         inst.adj.send_bytes = 2
         inst.will_close = False
         inst.last_activity = 0