Merge lp:~jameinel/bzr/2.5-conn-reset-socket-pipe-1047325 into lp:bzr/2.5

Proposed by John A Meinel
Status: Merged
Approved by: John A Meinel
Approved revision: 6514
Merged at revision: 6511
Proposed branch: lp:~jameinel/bzr/2.5-conn-reset-socket-pipe-1047325
Merge into: lp:bzr/2.5
Diff against target: 78 lines (+25/-5)
3 files modified
bzrlib/osutils.py (+5/-2)
bzrlib/smart/medium.py (+2/-2)
bzrlib/tests/test_osutils.py (+18/-1)
To merge this branch: bzr merge lp:~jameinel/bzr/2.5-conn-reset-socket-pipe-1047325
Reviewer Review Type Date Requested Status
Richard Wilbur Approve
Review via email: mp+123535@code.launchpad.net

Commit message

Treat socket.error(EPIPE) as a ConnectionReset (bug #1047325) so that clients will properly reconnect.

Description of the change

This updates osutils.send_all to translate more failures into errors.ConnectionReset.

I was originally going to do it at the higher level, but then I realized nobody who uses send_all really wants to be handling the raw IOError/socket.error sort of objects.
This will conflict a bit with my other osutils.send_all patch, but I'll handle resolving the conflicts.

To post a comment you must log in.
Revision history for this message
Richard Wilbur (richard-wilbur) wrote :

This looks good but I agree the integration with your other patch is very important. Should the other patch also treat a single 0 return value as a ConnectionReset?

+1

review: Approve
Revision history for this message
John A Meinel (jameinel) wrote :

Yes, I did change the other branch in the same way.

6514. By John A Meinel

Merge 2.5 including the other osutils.send_all fix.

Revision history for this message
John A Meinel (jameinel) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py 2012-09-11 07:26:51 +0000
+++ bzrlib/osutils.py 2012-09-11 08:42:19 +0000
@@ -2064,7 +2064,7 @@
2064# data at once.2064# data at once.
2065MAX_SOCKET_CHUNK = 64 * 10242065MAX_SOCKET_CHUNK = 64 * 1024
20662066
2067_end_of_stream_errors = [errno.ECONNRESET]2067_end_of_stream_errors = [errno.ECONNRESET, errno.EPIPE, errno.EINVAL]
2068for _eno in ['WSAECONNRESET', 'WSAECONNABORTED']:2068for _eno in ['WSAECONNRESET', 'WSAECONNABORTED']:
2069 _eno = getattr(errno, _eno, None)2069 _eno = getattr(errno, _eno, None)
2070 if _eno is not None:2070 if _eno is not None:
@@ -2136,7 +2136,10 @@
2136 while sent_total < byte_count:2136 while sent_total < byte_count:
2137 try:2137 try:
2138 sent = sock.send(buffer(bytes, sent_total, MAX_SOCKET_CHUNK))2138 sent = sock.send(buffer(bytes, sent_total, MAX_SOCKET_CHUNK))
2139 except socket.error, e:2139 except (socket.error, IOError), e:
2140 if e.args[0] in _end_of_stream_errors:
2141 raise errors.ConnectionReset(
2142 "Error trying to write to socket", e)
2140 if e.args[0] != errno.EINTR:2143 if e.args[0] != errno.EINTR:
2141 raise2144 raise
2142 else:2145 else:
21432146
=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py 2011-12-24 09:59:02 +0000
+++ bzrlib/smart/medium.py 2012-09-11 08:42:19 +0000
@@ -910,7 +910,7 @@
910 except IOError, e:910 except IOError, e:
911 if e.errno in (errno.EINVAL, errno.EPIPE):911 if e.errno in (errno.EINVAL, errno.EPIPE):
912 raise errors.ConnectionReset(912 raise errors.ConnectionReset(
913 "Error trying to write to subprocess:\n%s" % (e,))913 "Error trying to write to subprocess", e)
914 raise914 raise
915 self._report_activity(len(bytes), 'write')915 self._report_activity(len(bytes), 'write')
916916
@@ -1043,7 +1043,7 @@
10431043
1044class SmartClientSocketMedium(SmartClientStreamMedium):1044class SmartClientSocketMedium(SmartClientStreamMedium):
1045 """A client medium using a socket.1045 """A client medium using a socket.
1046 1046
1047 This class isn't usable directly. Use one of its subclasses instead.1047 This class isn't usable directly. Use one of its subclasses instead.
1048 """1048 """
10491049
10501050
=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py 2012-09-11 07:26:51 +0000
+++ bzrlib/tests/test_osutils.py 2012-09-11 08:42:19 +0000
@@ -819,9 +819,26 @@
819 self.assertEqual(None, osutils.safe_file_id(None))819 self.assertEqual(None, osutils.safe_file_id(None))
820820
821821
822
823class TestSendAll(tests.TestCase):822class TestSendAll(tests.TestCase):
824823
824 def test_send_with_disconnected_socket(self):
825 class DisconnectedSocket(object):
826 def __init__(self, err):
827 self.err = err
828 def send(self, content):
829 raise self.err
830 def close(self):
831 pass
832 # All of these should be treated as ConnectionReset
833 errs = []
834 for err_cls in (IOError, socket.error):
835 for errnum in osutils._end_of_stream_errors:
836 errs.append(err_cls(errnum))
837 for err in errs:
838 sock = DisconnectedSocket(err)
839 self.assertRaises(errors.ConnectionReset,
840 osutils.send_all, sock, 'some more content')
841
825 def test_send_with_no_progress(self):842 def test_send_with_no_progress(self):
826 # See https://bugs.launchpad.net/bzr/+bug/1047309843 # See https://bugs.launchpad.net/bzr/+bug/1047309
827 # It seems that paramiko can get into a state where it doesn't error,844 # It seems that paramiko can get into a state where it doesn't error,

Subscribers

People subscribed via source and target branches