osutils.send_all() loops endlessly sending 0 bytes if using paramiko for ssh

Bug #1047309 reported by John A Meinel
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fix Released
John A Meinel
Fix Released
John A Meinel

Bug Description

I'm investigating the ConnectionReset issues we've been seeing.

It appears that if you set BZR_SSH=paramiko and then run this:
python -c "import time; from bzrlib import breakin, branch, plugin, initialize; e = initialize(); e.__enter__(); plugin.load_plugins(); breakin.hook_debugger_to_signal();
b = branch.Branch.open('lp:~bzr-pqm/bzr/bzr.dev'); b.lock_read();
print b.last_revision(); b.unlock();
for i in range(35):
  print i
print b.last_revision()

Then the code loops endlessly in this loop:
def send_all(sock, bytes, report_activity=None):
    sent_total = 0
    byte_count = len(bytes)
    while sent_total < byte_count:
            sent = sock.send(buffer(bytes, sent_total, MAX_SOCKET_CHUNK))
        except socket.error, e:
            if e.args[0] != errno.EINTR:
            sent_total += sent
            report_activity(sent, 'write')

In particular sock.send() in this case returns '0' bytes sent. But it doesn't actually give an error. I think we should probably put a trap on allowing 0 bytes to be sent for more than X tries. I don't know if it is ever genuine for sock.send to return 0 bytes, but we shouldn't try to send forever and always send 0 bytes.

Tags: paramiko

Related branches

John A Meinel (jameinel)
summary: - ssh paramiko loops endlessly sending 0 bytes
+ osutils.send_all() loops endlessly sending 0 bytes if using paramiko for
+ ssh
Revision history for this message
John A Meinel (jameinel) wrote :

A first draft fix would say "if we have sent 0 bytes after X tries, fail". Looking at the man page, send() should block until the bytes requested are sent, or give other sorts of errors about the message size being too big, etc.

I would probably do something like:

num_zero_sends = 0
max_zero_sends = 3 # config entry?

while sent_bytes < ...:
  if sent == 0:
    num_zero_sends += 1
    if num_zero_sends > max_zero_sends:
        raise errors.ConnectionReset(...)
    num_zero_sends = 0

That allows us to have a transient '0' bytes sent at any point, but in the end
we must get forward progress or we will fail properly.

John A Meinel (jameinel)
Changed in bzr:
assignee: nobody → John A Meinel (jameinel)
John A Meinel (jameinel)
Changed in bzr:
status: Confirmed → Fix Released
milestone: none → 2.5.2
milestone: 2.5.2 → 2.6b3
Vincent Ladeuil (vila)
Changed in bzr:
milestone: 2.6b3 → 2.6.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers