Skip to content

Commit a9af01f

Browse files
committed
host.py: fix subprocess bug
There is an edge case we can hit when reading from a subprocess that will cause the read operation to hang indefinitely, which is exposed by openshift/dpu-operator@c517b55. We can reach a deadlock when reading from both of these buffers independently with a significant amount of output coming through. It makes more sense anyways to just read from proc.communicate() than manually reading from stdout and stderr. Signed-off-by: Salvatore Daniele <[email protected]>
1 parent a7e375f commit a9af01f

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

host.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,14 @@ def run(self, cmd: str, log_level: int = logging.DEBUG, env: dict[str, str] = os
245245
if not quiet:
246246
logger.log(log_level, f"running command {cmd} on {self._hostname}")
247247
if self.is_localhost():
248-
ret_val = self._run_local(cmd, env, quiet)
248+
ret_val = self._run_local(cmd, env)
249249
else:
250-
ret_val = self._run_remote(cmd, log_level, quiet)
250+
ret_val = self._run_remote(cmd, log_level)
251251

252252
logger.log(log_level, ret_val)
253253
return ret_val
254254

255-
def _run_local(self, cmd: str, env: dict[str, str], quiet: bool = False) -> Result:
255+
def _run_local(self, cmd: str, env: dict[str, str]) -> Result:
256256
args = shlex.split(cmd)
257257
pipe = subprocess.PIPE
258258
with subprocess.Popen(args, stdout=pipe, stderr=pipe, env=env) as proc:
@@ -262,13 +262,11 @@ def _run_local(self, cmd: str, env: dict[str, str], quiet: bool = False) -> Resu
262262
if proc.stderr is None:
263263
logger.info("Can't find stderr")
264264
sys.exit(-1)
265-
out = proc.stdout.read().decode("utf-8")
266-
err = proc.stderr.read().decode("utf-8")
267-
proc.communicate()
265+
out, err = proc.communicate()
268266
ret = proc.returncode
269-
return Result(out, err, ret)
267+
return Result(out.decode("utf-8"), err.decode("utf-8"), ret)
270268

271-
def _run_remote(self, cmd: str, log_level: int, quiet: bool = False) -> Result:
269+
def _run_remote(self, cmd: str, log_level: int) -> Result:
272270
def read_output(cmd: str, log_level: int) -> Result:
273271
assert self._host is not None
274272
_, stdout, stderr = self._host.exec_command(cmd)

0 commit comments

Comments
 (0)