Skip to content

Commit

Permalink
ssh: Prevent timeout from deadlock
Browse files Browse the repository at this point in the history
Using Popen.wait() on a process that has output sent to a pipe can
potentially deadlock if the process produces enough output to fill the
pipe, since it will stall and never terminate waiting for the pipe to
have more space.

Instead, use Popen.communicate() with the timeout parameter. This will
consume all output until EOF (preventing the process from stalling due
to a full pipe), and then check the return code. In the event of a
timeout error, Popen.communicate() doesn't loose any data, so it's safe
to call it again after the Popen.kill() in the exception handler.

This likely was done this way because the timeout parameter was new in
Python 3.3, but this shouldn't be a concern anymore

Signed-off-by: Joshua Watt <[email protected]>
  • Loading branch information
JoshuaWatt committed Sep 7, 2023
1 parent 2dcc631 commit f9ca024
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions labgrid/util/ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ def _start_own_master(self):
)

try:
if self._master.wait(timeout=connect_timeout) != 0:
stdout, stderr = self._master.communicate()
stdout, stderr = self._master.communicate(timeout=connect_timeout)
if self._master.returncode != 0:
raise ExecutionError(
f"failed to connect to {self.host} with args {args}, returncode={self._master.returncode} {stdout},{stderr}" # pylint: disable=line-too-long
)
Expand Down

0 comments on commit f9ca024

Please sign in to comment.