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
1=== modified file 'NEWS'
2--- NEWS 2009-06-05 23:21:51 +0000
3+++ NEWS 2009-06-09 03:35:18 +0000
4@@ -66,6 +66,9 @@
5 * Better message in ``bzr add`` output suggesting using ``bzr ignored`` to
6 see which files can also be added. (Jason Spashett, #76616)
7
8+* ``bzr serve`` on Windows no longer displays a traceback simply because a
9+ TCP client disconnected. (Andrew Bennetts)
10+
11 * Clarify the rules for locking and fallback repositories. Fix bugs in how
12 ``RemoteRepository`` was handling fallbacks along with the
13 ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)
14
15=== modified file 'bzrlib/smart/medium.py'
16--- bzrlib/smart/medium.py 2009-05-05 22:11:41 +0000
17+++ bzrlib/smart/medium.py 2009-06-09 03:35:18 +0000
18@@ -285,11 +285,8 @@
19 self._push_back(protocol.unused_data)
20
21 def _read_bytes(self, desired_count):
22- # We ignore the desired_count because on sockets it's more efficient to
23- # read large chunks (of _MAX_READ_SIZE bytes) at a time.
24- bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
25- self._report_activity(len(bytes), 'read')
26- return bytes
27+ return _read_bytes_from_socket(
28+ self.socket.recv, desired_count, self._report_activity)
29
30 def terminate_due_to_error(self):
31 # TODO: This should log to a server log file, but no such thing
32@@ -884,19 +881,8 @@
33 """See SmartClientMedium.read_bytes."""
34 if not self._connected:
35 raise errors.MediumNotConnected(self)
36- # We ignore the desired_count because on sockets it's more efficient to
37- # read large chunks (of _MAX_READ_SIZE bytes) at a time.
38- try:
39- bytes = osutils.until_no_eintr(self._socket.recv, _MAX_READ_SIZE)
40- except socket.error, e:
41- if len(e.args) and e.args[0] == errno.ECONNRESET:
42- # Callers expect an empty string in that case
43- return ''
44- else:
45- raise
46- else:
47- self._report_activity(len(bytes), 'read')
48- return bytes
49+ return _read_bytes_from_socket(
50+ self._socket.recv, count, self._report_activity)
51
52
53 class SmartClientStreamMediumRequest(SmartClientMediumRequest):
54@@ -938,3 +924,20 @@
55 """
56 self._medium._flush()
57
58+
59+def _read_bytes_from_socket(sock, desired_count, report_activity):
60+ # We ignore the desired_count because on sockets it's more efficient to
61+ # read large chunks (of _MAX_READ_SIZE bytes) at a time.
62+ try:
63+ bytes = osutils.until_no_eintr(sock, _MAX_READ_SIZE)
64+ except socket.error, e:
65+ if len(e.args) and e.args[0] in (errno.ECONNRESET, 10054):
66+ # The connection was closed by the other side. Callers expect an
67+ # empty string to signal end-of-stream.
68+ bytes = ''
69+ else:
70+ raise
71+ else:
72+ report_activity(len(bytes), 'read')
73+ return bytes
74+