Skip to content

Conversation

@jaysoffian
Copy link

Both sendall and sendall_stderr call send inside a loop and use its result to advance their buffer. If the channel is closed, send returns 0 which causes both sendall methods to go into an infinite loop. Prevent this from occurring by using the open_only decorator.

I'm not sure this is the best fix since I haven't verified whether the channel can be closed after the sending begins. If so, then both sendall routines should instead raise an exception if send returns 0.

Both sendall and sendall_stderr call send inside a loop and use its result
to advance their buffer. If the channel is closed, send returns 0 which
causes both sendall methods to go into an infinite loop. Prevent this from
occurring by using the open_only decorator.

I'm not sure this is the best fix since I haven't verified whether the channel
can be closed after the sending begins. If so, then both sendall routines should
instead raise an exception if send returns 0.
@jaysoffian jaysoffian changed the title channel: prevent infinite loop sending closed channel channel: prevent infinite loop sending to closed channel Sep 18, 2022
jaysoffian added a commit to jaysoffian/fab-classic that referenced this pull request Sep 18, 2022
When attempting to use parallel mode along with sudo, fabric was causing
paramiko-ng to go into an infinite loop. The reason was due to a combination of
fabric calling sendall() along with it misdetecting EOF on the input and closing
the output channel. The fix for paramiko-ng going into an infinite loop is here:

ploxiln/paramiko-ng#140

This commit fixes the root cause in fabric where it (apparently) erroneously 
detects the input channel as being closed when it is not.
@ploxiln
Copy link
Owner

ploxiln commented Oct 24, 2022

I'm unsure of this particular solution, because:

  • @open_only checks for both eof_sent and eof_received. stdin and stdout are independent. These send methods should probably only check eof_sent?
  • These methods call send() which calls _send(), which raises an exception if the socket is closed (another thing @open_only checks). But it does seem that _send() can just return 0 if eof_sent, maybe just that should be checked.
  • Maybe the check should be inside the while loop in these send_all methods, instead of on the outside, in case the EOF occurs after starting the method?

@jaysoffian
Copy link
Author

I'm unsure of this particular solution, because:

These are all good questions, but I was stuck with an infinite loop and this seemed like a minimally invasive fix at the time. I'll work on a more targeted fix that just avoids the infinite loop and hopefully has no other side-effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants