diff --git a/src/openjd/adaptor_runtime/process/_logging_subprocess.py b/src/openjd/adaptor_runtime/process/_logging_subprocess.py index 0073c26..cc9833d 100644 --- a/src/openjd/adaptor_runtime/process/_logging_subprocess.py +++ b/src/openjd/adaptor_runtime/process/_logging_subprocess.py @@ -64,6 +64,13 @@ def __init__( stderr=subprocess.PIPE, stdout=subprocess.PIPE, encoding=encoding, + # Decode the subprocess output leniently. DCC batch processes may emit + # locale-encoded (non-UTF-8) bytes on stdout/stderr; with the default strict + # error handling a single undecodable byte raises UnicodeDecodeError in the + # stream-reader thread, killing it. Once the reader thread is gone the pipe is + # no longer drained, the subprocess blocks on a full stdout pipe, and the job + # deadlocks. Replacing undecodable bytes keeps the reader alive. + errors="replace", cwd=startup_directory, ) if OSName.is_windows(): # pragma: is-posix diff --git a/test/openjd/adaptor_runtime/integ/process/test_integration_logging_subprocess.py b/test/openjd/adaptor_runtime/integ/process/test_integration_logging_subprocess.py index 87d20e4..4be420a 100644 --- a/test/openjd/adaptor_runtime/integ/process/test_integration_logging_subprocess.py +++ b/test/openjd/adaptor_runtime/integ/process/test_integration_logging_subprocess.py @@ -151,6 +151,37 @@ def test_log_levels(self, log_level: int, caplog): assert any(r.message == message and r.levelno == _STDERR_LEVEL for r in records) + @pytest.mark.timeout(10) + def test_non_utf8_output_does_not_kill_reader(self, caplog): + """ + A subprocess that writes a non-UTF-8 (locale-encoded) byte to stdout must not kill the + stream-reader thread. If the reader thread dies, lines emitted after the undecodable byte + are never logged and the stdout pipe stops being drained, which can deadlock the subprocess. + This is a regression test for that failure mode; the undecodable byte must be tolerated and + subsequent output must still be logged. + """ + # GIVEN + caplog.set_level(_STDOUT_LEVEL) + # 0xc7 is "Ç" in cp1252 and is not a valid standalone UTF-8 byte. + script = ( + "import sys; " + "sys.stdout.buffer.write(b'before\\n'); " + "sys.stdout.buffer.write(b'bad \\xc7 byte\\n'); " + "sys.stdout.buffer.write(b'after\\n'); " + "sys.stdout.buffer.flush()" + ) + + # WHEN + p = LoggingSubprocess(args=[sys.executable, "-c", script]) + p.wait() + p._cleanup_io_threads() + + # THEN + # "after" is only logged if the reader thread survived the undecodable byte. + assert "after" in caplog.text + # The undecodable byte is replaced rather than raising. + assert "�" in caplog.text + def test_executable_not_found(self): """When calling LoggingSubprocess with a missing executable, FileNotFoundError will be raised""" args = ["missing_executable"] diff --git a/test/openjd/adaptor_runtime/unit/process/test_logging_subprocess.py b/test/openjd/adaptor_runtime/unit/process/test_logging_subprocess.py index 8c81d3a..c55899e 100644 --- a/test/openjd/adaptor_runtime/unit/process/test_logging_subprocess.py +++ b/test/openjd/adaptor_runtime/unit/process/test_logging_subprocess.py @@ -69,6 +69,7 @@ def test_process_creation(self, mock_popen: mock.Mock, mock_stream_logger: mock. popen_params = dict( args=args, encoding="utf-8", + errors="replace", stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -482,6 +483,7 @@ def test_startup_directory_default(self, mock_popen_autospec: mock.Mock): stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", + errors="replace", cwd=None, ) if OSName.is_windows(): @@ -504,6 +506,7 @@ def test_start_directory(self, mock_popen_autospec: mock.Mock): stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", + errors="replace", cwd="startup_dir", ) if OSName.is_windows():