Rev 6510: Fix bug #1047309. Treat a series of no-bytes-sent as a ECONNRESET failure. in http://bazaar.launchpad.net/~jameinel/bzr/2.5-unending-sendall-1047309

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.5-unending-sendall-1047309

------------------------------------------------------------
revno: 6510
revision-id: john at arbash-meinel.com-20120907131502-q1t9i715sun447di
parent: pqm at pqm.ubuntu.com-20120907110410-fqh06iqg61pknazl
fixes bug: https://launchpad.net/bugs/1047309
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.5-unending-sendall-1047309
timestamp: Fri 2012-09-07 17:15:02 +0400
message:
  Fix bug #1047309. Treat a series of no-bytes-sent as a ECONNRESET failure.
-------------- next part --------------
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2012-02-28 04:58:14 +0000
+++ b/bzrlib/osutils.py	2012-09-07 13:15:02 +0000
@@ -2118,6 +2118,12 @@
     return b
 
 
+# Consider making this configurable, but right now that seems very much YAGNI
+# This is how many 0's we can get in-a-row. If we manage to send some data
+# after a while, this number gets reset.
+_max_no_content_sends = 3
+
+
 def send_all(sock, bytes, report_activity=None):
     """Send all bytes on a socket.
 
@@ -2132,6 +2138,7 @@
         Transport._report_activity
     """
     sent_total = 0
+    no_content_count = 0
     byte_count = len(bytes)
     while sent_total < byte_count:
         try:
@@ -2140,8 +2147,17 @@
             if e.args[0] != errno.EINTR:
                 raise
         else:
+            if sent == 0:
+                no_content_count += 1
+                if no_content_count > _max_no_content_sends:
+                    raise IOError(errno.ECONNRESET,
+                        'Sending to %s returned 0 bytes sent %d times in a row'
+                        % (sock, no_content_count))
+            else:
+                no_content_count = 0
             sent_total += sent
-            report_activity(sent, 'write')
+            if report_activity is not None:
+                report_activity(sent, 'write')
 
 
 def connect_socket(address):

=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py	2012-02-28 04:58:14 +0000
+++ b/bzrlib/tests/test_osutils.py	2012-09-07 13:15:02 +0000
@@ -819,6 +819,44 @@
         self.assertEqual(None, osutils.safe_file_id(None))
 
 
+
+class TestSendAll(tests.TestCase):
+
+    def test_send_with_no_progress(self):
+        # See https://bugs.launchpad.net/bzr/+bug/1047309
+        # It seems that paramiko can get into a state where it doesn't error,
+        # but it returns 0 bytes sent for requests over and over again.
+        class NoSendingSocket(object):
+            def __init__(self):
+                self.call_count = 0
+            def send(self, bytes):
+                self.call_count += 1
+                if self.call_count > 100:
+                    # Prevent the test suite from hanging
+                    raise RuntimeError('too many calls')
+                return 0
+        sock = NoSendingSocket()
+        self.assertRaises(IOError, osutils.send_all, sock, 'content')
+
+    def test_send_minimal_progress(self):
+        # Even if we occasionally get 0 bytes sent, we still progress and
+        # finish
+        class SlowSendingSocket(object):
+            def __init__(self):
+                self.call_count = 0
+            def send(self, bytes):
+                self.call_count += 1
+                if self.call_count > 100:
+                    # Prevent the test suite from hanging
+                    raise RuntimeError('too many calls')
+                if self.call_count % 3 == 0:
+                    return 0
+                return 1
+        sock = SlowSendingSocket()
+        osutils.send_all(sock, 'a reasonable amount of content')
+        self.assertEqual(44, sock.call_count)
+
+
 class TestPosixFuncs(tests.TestCase):
     """Test that the posix version of normpath returns an appropriate path
        when used with 2 leading slashes."""

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2012-09-06 12:05:27 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2012-09-07 13:15:02 +0000
@@ -39,6 +39,13 @@
   extracted texts from the repository. (Just an ordering constraint on how
   they consumed the stream.) (John Arbash Meinel, #1046284)
 
+* ``osutils.send_all`` now detects if we get a series of zero bytes sent,
+  and fails with a ECONNRESET. It seems if paramiko gets disconnected, it
+  will get into a state where it returns 0 bytes sent, but doesn't raise
+  an error. This change allows us to get a couple hiccups of no content
+  sent, but if it is consistent, we will consider it to be a failure.
+  (John Arbash Meinel, #1047309)
+
 * Revert use of --no-tty when gpg signing commits. (Jelmer Vernooij, #1014570)
 
 * Some small bug fixes wrt lightweight checkouts and remote repositories.