Merge lp:~spiv/bzr/win-serve-error into lp:~bzr/bzr/trunk-old

Proposed by Andrew Bennetts on 2009-05-27
Status: Merged
Approved by: Ian Clatworthy on 2009-05-28
Approved revision: 4383
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/win-serve-error
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 74 lines
To merge this branch: bzr merge lp:~spiv/bzr/win-serve-error
Reviewer Review Type Date Requested Status
Ian Clatworthy 2009-05-27 Approve on 2009-05-28
Review via email: mp+6827@code.launchpad.net
To post a comment you must log in.
Robert Collins (lifeless) wrote :

On Wed, 2009-05-27 at 15:25 +0000, Andrew Bennetts wrote:
> - bytes = osutils.until_no_eintr(self.socket.recv,
> _MAX_READ_SIZE)
> + try:
> + bytes = osutils.until_no_eintr(self.socket.recv,
> _MAX_READ_SIZE)
> + except socket.error, e:
> + if e.args[0] in (errno.ECONNRESET, 10054):
> + # The connection was closed by the other side.
> + return ''
> + raise
> self._report_activity(len(bytes), 'read')
> return bytes

How about
- bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
+ try:
+ bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
+ except socket.error, e:
+ if e.args[0] in (errno.ECONNRESET, 10054):
+ # The connection was closed by the other side.
+ bytes = ''
+ else:
+ raise
         self._report_activity(len(bytes), 'read')
         return bytes

This avoids having a different code path exiting the function and may
scale better as other cases need to be caught.

-Rob

Andrew Bennetts (spiv) wrote :

Robert Collins wrote:
[...]
> How about
[...]
> + if e.args[0] in (errno.ECONNRESET, 10054):
> + # The connection was closed by the other side.
> + bytes = ''
[...]
> This avoids having a different code path exiting the function and may
> scale better as other cases need to be caught.

Yeah, that's better. Thanks!

-Andrew.

Ian Clatworthy (ian-clatworthy) wrote :

This looks fine to land after making the tweak Robert suggested.

review: Approve
lp:~spiv/bzr/win-serve-error updated on 2009-06-09
4384. By Andrew Bennetts on 2009-06-09

Refactor duplicated SmartServerSocketStreamMedium._read_bytes and SmartTCPClientMedium._read_bytes to share a common implementation with the best parts of both. Includes Robert's review feedback.

4385. By Andrew Bennetts on 2009-06-09

Merge bzr.dev.

4386. By Andrew Bennetts on 2009-06-09

Add NEWS entry.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-06-05 23:21:51 +0000
+++ NEWS 2009-06-09 03:35:18 +0000
@@ -66,6 +66,9 @@
66* Better message in ``bzr add`` output suggesting using ``bzr ignored`` to66* Better message in ``bzr add`` output suggesting using ``bzr ignored`` to
67 see which files can also be added. (Jason Spashett, #76616)67 see which files can also be added. (Jason Spashett, #76616)
6868
69* ``bzr serve`` on Windows no longer displays a traceback simply because a
70 TCP client disconnected. (Andrew Bennetts)
71
69* Clarify the rules for locking and fallback repositories. Fix bugs in how72* Clarify the rules for locking and fallback repositories. Fix bugs in how
70 ``RemoteRepository`` was handling fallbacks along with the73 ``RemoteRepository`` was handling fallbacks along with the
71 ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)74 ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)
7275
=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py 2009-05-05 22:11:41 +0000
+++ bzrlib/smart/medium.py 2009-06-09 03:35:18 +0000
@@ -285,11 +285,8 @@
285 self._push_back(protocol.unused_data)285 self._push_back(protocol.unused_data)
286286
287 def _read_bytes(self, desired_count):287 def _read_bytes(self, desired_count):
288 # We ignore the desired_count because on sockets it's more efficient to288 return _read_bytes_from_socket(
289 # read large chunks (of _MAX_READ_SIZE bytes) at a time.289 self.socket.recv, desired_count, self._report_activity)
290 bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
291 self._report_activity(len(bytes), 'read')
292 return bytes
293290
294 def terminate_due_to_error(self):291 def terminate_due_to_error(self):
295 # TODO: This should log to a server log file, but no such thing292 # TODO: This should log to a server log file, but no such thing
@@ -884,19 +881,8 @@
884 """See SmartClientMedium.read_bytes."""881 """See SmartClientMedium.read_bytes."""
885 if not self._connected:882 if not self._connected:
886 raise errors.MediumNotConnected(self)883 raise errors.MediumNotConnected(self)
887 # We ignore the desired_count because on sockets it's more efficient to884 return _read_bytes_from_socket(
888 # read large chunks (of _MAX_READ_SIZE bytes) at a time.885 self._socket.recv, count, self._report_activity)
889 try:
890 bytes = osutils.until_no_eintr(self._socket.recv, _MAX_READ_SIZE)
891 except socket.error, e:
892 if len(e.args) and e.args[0] == errno.ECONNRESET:
893 # Callers expect an empty string in that case
894 return ''
895 else:
896 raise
897 else:
898 self._report_activity(len(bytes), 'read')
899 return bytes
900886
901887
902class SmartClientStreamMediumRequest(SmartClientMediumRequest):888class SmartClientStreamMediumRequest(SmartClientMediumRequest):
@@ -938,3 +924,20 @@
938 """924 """
939 self._medium._flush()925 self._medium._flush()
940926
927
928def _read_bytes_from_socket(sock, desired_count, report_activity):
929 # We ignore the desired_count because on sockets it's more efficient to
930 # read large chunks (of _MAX_READ_SIZE bytes) at a time.
931 try:
932 bytes = osutils.until_no_eintr(sock, _MAX_READ_SIZE)
933 except socket.error, e:
934 if len(e.args) and e.args[0] in (errno.ECONNRESET, 10054):
935 # The connection was closed by the other side. Callers expect an
936 # empty string to signal end-of-stream.
937 bytes = ''
938 else:
939 raise
940 else:
941 report_activity(len(bytes), 'read')
942 return bytes
943