hpss SmartSSHClientMedium doesn't handle eintr

Bug #341535 reported by Martin Pool on 2009-03-12
Affects Status Importance Assigned to Milestone

Bug Description

Thu 2009-03-12 15:14:44 +1000
0.062 bzr arguments: [u'push']
0.104 looking for plugins in /home/mbp/.bazaar/plugins
0.294 looking for plugins in /home/mbp/bzr/progress-fetch/bzrlib/plugins
0.333 looking for plugins in /usr/lib/python2.6/dist-packages/bzrlib/plugins
0.334 Plugin name gtk already loaded
0.334 Plugin name launchpad already loaded
0.335 Plugin name netrc_credential_store already loaded
0.385 encoding stdout as sys.stdout encoding 'UTF-8'
9.864 hpss: Built a new medium: SmartSSHClientMedium
9.865 hpss call: 'BzrDir.open', '~mbp/bzr/progress-fetch/'
9.865 (to bzr+ssh://bazaar.launchpad.net/%7Embp/bzr/progress-fetch/)
9.957 ssh implementation is OpenSSH
16.034 Traceback (most recent call last):
  File "/home/mbp/bzr/progress-fetch/bzrlib/commands.py", line 716, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/home/mbp/bzr/progress-fetch/bzrlib/commands.py", line 911, in run_bzr
    ret = run(*run_argv)
  File "/home/mbp/bzr/progress-fetch/bzrlib/commands.py", line 547, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/mbp/bzr/progress-fetch/bzrlib/builtins.py", line 1033, in run
  File "/home/mbp/bzr/progress-fetch/bzrlib/push.py", line 48, in _show_push_branch
    dir_to = bzrdir.BzrDir.open_from_transport(to_transport)
  File "/home/mbp/bzr/progress-fetch/bzrlib/bzrdir.py", line 832, in open_from_transport
    return format.open(transport, _found=True)
  File "/home/mbp/bzr/progress-fetch/bzrlib/bzrdir.py", line 1834, in open
    return self._open(transport)
  File "/home/mbp/bzr/progress-fetch/bzrlib/bzrdir.py", line 2806, in _open
    return remote.RemoteBzrDir(transport, self)
  File "/home/mbp/bzr/progress-fetch/bzrlib/remote.py", line 110, in __init__
    response = self._call('BzrDir.open', path)
  File "/home/mbp/bzr/progress-fetch/bzrlib/remote.py", line 54, in _call
    return self._client.call(method, *args)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/client.py", line 129, in call
    result, protocol = self.call_expecting_body(method, *args)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/client.py", line 142, in call_expecting_body
    method, args, expect_response_body=True)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/client.py", line 90, in _call_and_read_response
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/message.py", line 297, in read_response_tuple
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/message.py", line 264, in _wait_for_response_args
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/message.py", line 277, in _read_more
    bytes = self._medium_request.read_bytes(next_read_size)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/medium.py", line 459, in read_bytes
    return self._read_bytes(count)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/medium.py", line 471, in _read_bytes
    return self._medium.read_bytes(count)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/medium.py", line 149, in read_bytes
    return self._read_bytes(bytes_to_read)
  File "/home/mbp/bzr/progress-fetch/bzrlib/smart/medium.py", line 787, in _read_bytes
    bytes = self._read_from.read(bytes_to_read)
IOError: [Errno 4] Interrupted system call

Related branches

Andrew Bennetts (spiv) wrote :

The way to fix this is to use bzrlib.osutils.until_no_eintr in the SmartSSHClientMedium. The SmartTCPClientMedium already does this. The change should be simple.

Changed in bzr:
status: New → Confirmed
Martin Pool (mbp) on 2009-09-01
Changed in bzr:
importance: Undecided → Medium
status: Confirmed → In Progress
assignee: nobody → Martin Pool (mbp)
Martin Pool (mbp) wrote :

OK, I have a patch that scatters some until_no_eintr calls around, in http://sourcefrog.net/tmp/20090901-341535-eintr.diff

I'm starting to feel that we should in fact have a file decorator that makes it handle eintr, but I didn't quite get to writing one.

Martin Pool (mbp) wrote :

It would be nice to add tests for this, but not easy as things stand, so I think it's worth merging without it.

In interactive testing, this does basically work so that you can interrupt the process and have it continue. However, you do have to say "kill -quit 1234" from another shell, because just pressing C-\ sends the signal to the whole process group(?) and that kills off ssh as well, so you can't continue.

Martin Pool (mbp) wrote :

poolie: spiv, if i thought there would be more cases then i'd do the object
poolie: is that the package in a distro?
spiv: If the underlying objects weren't private to the SmartMedium classes, that would be a pretty strong case for decorating.
spiv: poolie: right.
poolie: it's true it's more than 3 cases, strictly speaking
poolie: maybe i'll propose just this for now
spiv: poolie: So, I'm happy with the patch as is. If you feel like writing the decorator I'm happy to look at that page too :)
spiv: But it's not a big deal to postpone that refactoring indefinitely.


Martin Pool (mbp) wrote :

See also bug 162502 for accidentally killing ssh.

Martin Pool (mbp) on 2009-09-02
Changed in bzr:
status: In Progress → Fix Committed
milestone: none → 2.0rc2
Martin Pool (mbp) on 2009-09-03
Changed in bzr:
status: Fix Committed → Fix Released
John A Meinel (jameinel) on 2009-10-14
Changed in bzr:
milestone: 2.0.0 → 2.1.0b1
Andrew Bennetts (spiv) wrote :

Reopening because until_no_eintr isn't safe to use in many places, so this "fix" has been partially reverted in lp:bzr r5117.

Changed in bzr:
assignee: Martin Pool (mbp) → nobody
status: Fix Released → Confirmed
summary: - hpss SmartMedium doesn't handle eintr
+ hpss SmartSSHClientMedium doesn't handle eintr
Andrew Bennetts (spiv) wrote :

There's some discussion of this at <https://code.launchpad.net/~spiv/bzr/no_until_no_eintr/+merge/21699>, and earlier merge proposals.

tags: removed: easy
Vincent Ladeuil (vila) on 2010-09-17
Changed in bzr:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers