From 06f6d835370aa7c1cb53f781a30d823fa09b5bdd Mon Sep 17 00:00:00 2001 From: Arkadiusz Baczek <86051266+abaczek@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:32:18 +0000 Subject: [PATCH 1/2] fix: discard unconsumed rshell server output Signed-off-by: Arkadiusz Baczek <86051266+abaczek@users.noreply.github.com> --- README.md | 4 +++ mfd_connect/rshell.py | 8 +++-- tests/unit/test_mfd_connect/test_rshell.py | 34 ++++++++++++++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 6f2bd46..2857644 100644 --- a/README.md +++ b/README.md @@ -1166,6 +1166,10 @@ If code cannot establish connection, it will start deployment of python using [D `RShellConnection` is a connection type that leverages `httplib.client` library to establish and manage connections using RESTful API over HTTP protocol in EFI Shell environment. `rshell_client.py` must be present on the EFI Shell target system. +When `RShellConnection` starts a local server automatically, its stdout and stderr are discarded +to prevent blocking on unread pipes. Command results still return through the HTTP API. +For server diagnostics, start the server manually with output directed to a terminal or a managed log file. + `rshell_server.py` is a server script that needs to be executed on the host machine to facilitate communication between the host and the EFI Shell target system. Server works on queue with address ip -> command to execute, it provides a RESTful API that allows the host to send commands and receive responses from the EFI Shell: * `/execute_command` - Endpoint to execute commands on the EFI Shell target system: diff --git a/mfd_connect/rshell.py b/mfd_connect/rshell.py index c5de937..1237968 100644 --- a/mfd_connect/rshell.py +++ b/mfd_connect/rshell.py @@ -125,14 +125,18 @@ def _run_server(self) -> LocalProcess: """Run RShell server locally.""" conn = LocalConnection() conn.enable_sudo() - process: LocalProcess = conn.start_process(f"{conn.modules().sys.executable} -m mfd_connect.rshell_server") + process: LocalProcess = conn.start_process( + f"{conn.modules().sys.executable} -m mfd_connect.rshell_server", + discard_stdout=True, + discard_stderr=True, + ) conn.disable_sudo() timeout = TimeoutCounter(1) while not timeout: if not process.running: raise RuntimeError( f"RShell server failed to start. Exit code: {process.return_code}\n" - f"STDOUT: {process.stdout_text}\nSTDERR: {process.stderr_text}" + "Server stdout and stderr are discarded. Run the server manually to inspect its diagnostics." ) return process diff --git a/tests/unit/test_mfd_connect/test_rshell.py b/tests/unit/test_mfd_connect/test_rshell.py index 9ef8621..89cb8bd 100644 --- a/tests/unit/test_mfd_connect/test_rshell.py +++ b/tests/unit/test_mfd_connect/test_rshell.py @@ -5,6 +5,7 @@ import sys import types import runpy +import shlex from pathlib import Path from subprocess import CalledProcessError from unittest.mock import Mock, patch @@ -15,6 +16,7 @@ from mfd_typing.os_values import OSBitness, OSName, OSType from mfd_connect.base import ConnectionCompletedProcess +from mfd_connect.local import LocalConnection from mfd_connect.exceptions import ConnectionCalledProcessError, OsNotSupported from mfd_connect.rshell import ( PLATFORM_POWER_TRANSITION_DELAY_SECONDS, @@ -147,7 +149,29 @@ def __bool__(self): result = rshell._run_server() assert result == process - conn.start_process.assert_called_once_with("python -m mfd_connect.rshell_server") + conn.start_process.assert_called_once_with( + "python -m mfd_connect.rshell_server", discard_stdout=True, discard_stderr=True + ) + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX pipe regression") + def test_run_server_discards_output_larger_than_pipe_capacity(self, rshell, mocker, tmp_path): + script = tmp_path / "verbose_server.py" + script.write_text("import sys\nsys.stdout.write('x' * 1048576)\nsys.stderr.write('x' * 1048576)\n") + conn = LocalConnection() + start_process = conn.start_process + command = f"{shlex.quote(sys.executable)} {shlex.quote(str(script))}" + mocker.patch.object(conn, "enable_sudo") + mocker.patch.object(conn, "disable_sudo") + mocker.patch.object(conn, "start_process", side_effect=lambda _, **kwargs: start_process(command, **kwargs)) + mocker.patch("mfd_connect.rshell.LocalConnection", return_value=conn) + mocker.patch("mfd_connect.rshell.TimeoutCounter", return_value=True) + + process = rshell._run_server() + try: + assert process.wait(timeout=5) == 0 + finally: + if process.running: + process.stop(wait=5) def test_run_server_timeout_loop_executes(self, rshell, mocker): """Test that the timeout loop in _run_server executes at least once.""" @@ -186,8 +210,10 @@ def test_run_server_server_not_running_raises_error(self, rshell, mocker): process = mocker.Mock() process.running = False # Process crashed process.return_code = 1 - process.stdout_text = "stdout output" - process.stderr_text = "stderr output" + stdout = mocker.PropertyMock(side_effect=AssertionError("Discarded stdout must not be read")) + stderr = mocker.PropertyMock(side_effect=AssertionError("Discarded stderr must not be read")) + type(process).stdout_text = stdout + type(process).stderr_text = stderr conn = mocker.Mock() conn.modules().sys.executable = "python" @@ -212,6 +238,8 @@ def __bool__(self): with pytest.raises(RuntimeError, match="RShell server failed to start.*Exit code: 1"): rshell._run_server() + stdout.assert_not_called() + stderr.assert_not_called() def test_type_checking_import_block_executes(self, monkeypatch): class _FakeBaseModel: From f7e47b655543a758fdd5b1ca06fb5a4497aa23fc Mon Sep 17 00:00:00 2001 From: Arkadiusz Baczek <86051266+abaczek@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:41:52 +0000 Subject: [PATCH 2/2] fix: avoid reading discarded rshell output during teardown Signed-off-by: Arkadiusz Baczek <86051266+abaczek@users.noreply.github.com> --- mfd_connect/rshell.py | 6 ------ tests/unit/test_mfd_connect/test_rshell.py | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/mfd_connect/rshell.py b/mfd_connect/rshell.py index 1237968..63390bd 100644 --- a/mfd_connect/rshell.py +++ b/mfd_connect/rshell.py @@ -393,9 +393,3 @@ def stop_server(self) -> None: raise RuntimeError("RShell server did not stop within timeout") logger.log(level=log_levels.MODULE_DEBUG, msg="RShell server stopped") - logger.log( - level=log_levels.MODULE_DEBUG, - msg=self.server_process.stdout_text - if self.server_process.stdout_text - else "RShell server had no stdout", - ) diff --git a/tests/unit/test_mfd_connect/test_rshell.py b/tests/unit/test_mfd_connect/test_rshell.py index 89cb8bd..5eec698 100644 --- a/tests/unit/test_mfd_connect/test_rshell.py +++ b/tests/unit/test_mfd_connect/test_rshell.py @@ -169,6 +169,14 @@ def test_run_server_discards_output_larger_than_pipe_capacity(self, rshell, mock process = rshell._run_server() try: assert process.wait(timeout=5) == 0 + rshell.server_process = process + mocker.patch( + "mfd_connect.rshell.requests.post", return_value=Mock(status_code=200, text="", headers={"rc": "0"}) + ) + kill_process = mocker.patch("mfd_connect.rshell.kill_process_by_pid") + mocker.patch("mfd_connect.rshell.TimeoutCounter", return_value=False) + rshell.disconnect(stop_client=True, stop_server=True) + kill_process.assert_called_once_with(conn, process.pid) finally: if process.running: process.stop(wait=5) @@ -536,7 +544,10 @@ def test_stop_server_with_and_without_process(self, rshell, mocker): server_process = mocker.Mock() server_process.pid = 1234 server_process.running = False - server_process.stdout_text = "server out" + stdout = mocker.PropertyMock(side_effect=AssertionError("Discarded stdout must not be read")) + stderr = mocker.PropertyMock(side_effect=AssertionError("Discarded stderr must not be read")) + type(server_process).stdout_text = stdout + type(server_process).stderr_text = stderr rshell.server_process = server_process rshell.stop_server() @@ -544,3 +555,5 @@ def test_stop_server_with_and_without_process(self, rshell, mocker): local_connection.enable_sudo.assert_called_once() kill_process_by_pid.assert_called_once_with(local_connection, 1234) local_connection.disable_sudo.assert_called_once() + stdout.assert_not_called() + stderr.assert_not_called()