Fix cases where ping would fire immediately after sending a packet.

If a first packet is sent and its answer received in time, then a second
packet is sent after more than PING_DELAY seconds, a ping will be emitted
right after that second packet, because _ping_time was still set.
This change resets it when there is no pending requests.

Also, as required to have a consistent view of _handlers, check that lock
is properly held in the MT variant of the class.

git-svn-id: https://svn.erp5.org/repos/neo/trunk@2033 71dcc9de-d417-0410-9af5-da40c76e7ee4

neo/connection.py

@@ -165,6 +165,12 @@ class Timeout(object):
     """ Keep track of connection-level timeouts """
 
     def __init__(self):
+        self.clear()
+
+    def clear(self):
+        """
+        There is no pending request, reset ping times.
+        """
         self._ping_time = None
         self._critical_time = None
 
@@ -446,7 +452,11 @@ class Connection(BaseConnection):
         """
         # check out packet and process it with current handler
         packet = self._queue.pop(0)
-        self._handlers.handle(packet)
+        handlers = self._handlers
+        handlers.handle(packet)
+        if not handlers.isPending():
+            # We are not expecting any other response, clear timeout
+            self._timeout.clear()
 
     def pending(self):
         return self.connector is not None and self.write_buf
@@ -695,3 +705,7 @@ class MTClientConnection(ClientConnection):
         finally:
             self.release()
 
+    @lockCheckWrapper
+    def process(self, *args, **kw):
+        return super(MTClientConnection, self).process(*args, **kw)
+

neo/tests/testConnection.py

@@ -999,7 +999,7 @@ class TestTimeout(NeoTestBase):
     def setUp(self):
         self.current = time()
         self.timeout = Timeout()
-        self.timeout.update(self.current)
+        self._updateAt(0)
         self.assertTrue(PING_DELAY > PING_TIMEOUT) # Sanity check
 
     def _checkAt(self, n, soft, hard):
@@ -1007,6 +1007,9 @@ class TestTimeout(NeoTestBase):
         self.assertEqual(soft, self.timeout.softExpired(at))
         self.assertEqual(hard, self.timeout.hardExpired(at))
 
+    def _updateAt(self, n):
+        self.timeout.update(self.current + n)
+
     def _refreshAt(self, n):
         self.timeout.refresh(self.current + n)
 
@@ -1030,6 +1033,13 @@ class TestTimeout(NeoTestBase):
         self._checkAt(PING_DELAY + 0.5, False, False)
         # ...but it will happen again after PING_DELAY after that answer
         self._checkAt(answer_time + PING_DELAY + 0.5, True, False)
+        # if there is no more pending requests, a clear will happen so next
+        # send doesn't immediately trigger a ping
+        self.timeout.clear()
+        new_request_time = answer_time + PING_DELAY * 2
+        self._updateAt(new_request_time)
+        self._checkAt(new_request_time + PING_DELAY - 0.5, False, False)
+        self._checkAt(new_request_time + PING_DELAY + 0.5, True, False)
 
     def testHardTimeout(self):
         """