From f9ca0243660f01567ac33a654ef5c54aff3683c0 Mon Sep 17 00:00:00 2001 From: Joshua Watt Date: Thu, 7 Sep 2023 08:36:05 -0600 Subject: [PATCH] ssh: Prevent timeout from deadlock 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 --- labgrid/util/ssh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/labgrid/util/ssh.py b/labgrid/util/ssh.py index 0f51bbce8..8df205510 100644 --- a/labgrid/util/ssh.py +++ b/labgrid/util/ssh.py @@ -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 )