From: Savannah Ostrowski Date: Sun, 14 Dec 2025 04:58:40 +0000 (-0800) Subject: GH-142591: Tachyon does not handle non-existent file/module (#142592) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f893e8f2568da975cfa577638362a0f97572076b;p=thirdparty%2FPython%2Fcpython.git GH-142591: Tachyon does not handle non-existent file/module (#142592) Co-authored-by: Pablo Galindo Salgado --- diff --git a/Lib/profiling/sampling/_sync_coordinator.py b/Lib/profiling/sampling/_sync_coordinator.py index be63dbe3e904..1a4af42588a3 100644 --- a/Lib/profiling/sampling/_sync_coordinator.py +++ b/Lib/profiling/sampling/_sync_coordinator.py @@ -5,6 +5,7 @@ This module is used internally by the sample profiler to coordinate the startup of target processes. It should not be called directly by users. """ +import importlib.util import os import sys import socket @@ -138,7 +139,7 @@ def _execute_module(module_name: str, module_args: List[str]) -> None: """ # Replace sys.argv to match how Python normally runs modules # When running 'python -m module args', sys.argv is ["__main__.py", "args"] - sys.argv = [f"__main__.py"] + module_args + sys.argv = ["__main__.py"] + module_args try: runpy.run_module(module_name, run_name="__main__", alter_sys=True) @@ -215,22 +216,31 @@ def main() -> NoReturn: # Set up execution environment _setup_environment(cwd) - # Signal readiness to profiler - _signal_readiness(sync_port) - - # Execute the target - if target_args[0] == "-m": - # Module execution + # Determine execution type and validate target exists + is_module = target_args[0] == "-m" + if is_module: if len(target_args) < 2: raise ArgumentError("Module name required after -m") - module_name = target_args[1] module_args = target_args[2:] - _execute_module(module_name, module_args) + + if importlib.util.find_spec(module_name) is None: + raise TargetError(f"Module not found: {module_name}") else: - # Script execution script_path = target_args[0] script_args = target_args[1:] + # Match the path resolution logic in _execute_script + check_path = script_path if os.path.isabs(script_path) else os.path.join(cwd, script_path) + if not os.path.isfile(check_path): + raise TargetError(f"Script not found: {script_path}") + + # Signal readiness to profiler + _signal_readiness(sync_port) + + # Execute the target + if is_module: + _execute_module(module_name, module_args) + else: _execute_script(script_path, script_args, cwd) except CoordinatorError as e: diff --git a/Lib/profiling/sampling/cli.py b/Lib/profiling/sampling/cli.py index 2bb4f31efe17..ffc80edc1f6e 100644 --- a/Lib/profiling/sampling/cli.py +++ b/Lib/profiling/sampling/cli.py @@ -1,10 +1,13 @@ """Command-line interface for the sampling profiler.""" import argparse +import importlib.util import os +import selectors import socket import subprocess import sys +import time from .sample import sample, sample_live from .pstats_collector import PstatsCollector @@ -92,6 +95,54 @@ def _parse_mode(mode_string): return mode_map[mode_string] +def _check_process_died(process): + """Check if process died and raise an error with stderr if available.""" + if process.poll() is None: + return # Process still running + + # Process died - try to get stderr for error message + stderr_msg = "" + if process.stderr: + try: + stderr_msg = process.stderr.read().decode().strip() + except (OSError, UnicodeDecodeError): + pass + + if stderr_msg: + raise RuntimeError(stderr_msg) + raise RuntimeError(f"Process exited with code {process.returncode}") + + +def _wait_for_ready_signal(sync_sock, process, timeout): + """Wait for the ready signal from the subprocess, checking for early death.""" + deadline = time.monotonic() + timeout + sel = selectors.DefaultSelector() + sel.register(sync_sock, selectors.EVENT_READ) + + try: + while True: + _check_process_died(process) + + remaining = deadline - time.monotonic() + if remaining <= 0: + raise socket.timeout("timed out") + + if not sel.select(timeout=min(0.1, remaining)): + continue + + conn, _ = sync_sock.accept() + try: + ready_signal = conn.recv(_RECV_BUFFER_SIZE) + finally: + conn.close() + + if ready_signal != _READY_MESSAGE: + raise RuntimeError(f"Invalid ready signal received: {ready_signal!r}") + return + finally: + sel.close() + + def _run_with_sync(original_cmd, suppress_output=False): """Run a command with socket-based synchronization and return the process.""" # Create a TCP socket for synchronization with better socket options @@ -117,24 +168,24 @@ def _run_with_sync(original_cmd, suppress_output=False): ) + tuple(target_args) # Start the process with coordinator - # Suppress stdout/stderr if requested (for live mode) + # When suppress_output=True (live mode), capture stderr so we can + # report errors if the process dies before signaling ready. + # When suppress_output=False (normal mode), let stderr inherit so + # script errors print to the terminal. popen_kwargs = {} if suppress_output: popen_kwargs["stdin"] = subprocess.DEVNULL popen_kwargs["stdout"] = subprocess.DEVNULL - popen_kwargs["stderr"] = subprocess.DEVNULL + popen_kwargs["stderr"] = subprocess.PIPE process = subprocess.Popen(cmd, **popen_kwargs) try: - # Wait for ready signal with timeout - with sync_sock.accept()[0] as conn: - ready_signal = conn.recv(_RECV_BUFFER_SIZE) + _wait_for_ready_signal(sync_sock, process, _SYNC_TIMEOUT) - if ready_signal != _READY_MESSAGE: - raise RuntimeError( - f"Invalid ready signal received: {ready_signal!r}" - ) + # Close stderr pipe if we were capturing it + if process.stderr: + process.stderr.close() except socket.timeout: # If we timeout, kill the process and raise an error @@ -632,6 +683,25 @@ def _handle_attach(args): def _handle_run(args): """Handle the 'run' command.""" + # Validate target exists before launching subprocess + if args.module: + # Temporarily add cwd to sys.path so we can find modules in the + # current directory, matching the coordinator's behavior + cwd = os.getcwd() + added_cwd = False + if cwd not in sys.path: + sys.path.insert(0, cwd) + added_cwd = True + try: + if importlib.util.find_spec(args.target) is None: + sys.exit(f"Error: Module not found: {args.target}") + finally: + if added_cwd: + sys.path.remove(cwd) + else: + if not os.path.exists(args.target): + sys.exit(f"Error: Script not found: {args.target}") + # Check if live mode is requested if args.live: _handle_live_run(args) @@ -644,7 +714,10 @@ def _handle_run(args): cmd = (sys.executable, args.target, *args.args) # Run with synchronization - process = _run_with_sync(cmd, suppress_output=False) + try: + process = _run_with_sync(cmd, suppress_output=False) + except RuntimeError as e: + sys.exit(f"Error: {e}") # Use PROFILING_MODE_ALL for gecko format mode = ( @@ -732,7 +805,10 @@ def _handle_live_run(args): cmd = (sys.executable, args.target, *args.args) # Run with synchronization, suppressing output for live mode - process = _run_with_sync(cmd, suppress_output=True) + try: + process = _run_with_sync(cmd, suppress_output=True) + except RuntimeError as e: + sys.exit(f"Error: {e}") mode = _parse_mode(args.mode) diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_cli.py b/Lib/test/test_profiling/test_sampling_profiler/test_cli.py index e1892ec91559..330f47ad7ed4 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_cli.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_cli.py @@ -13,7 +13,9 @@ except ImportError: "Test only runs when _remote_debugging is available" ) -from test.support import is_emscripten +from test.support import is_emscripten, requires_subprocess + +from profiling.sampling.cli import main class TestSampleProfilerCLI(unittest.TestCase): @@ -62,6 +64,7 @@ class TestSampleProfilerCLI(unittest.TestCase): self.assertEqual(coordinator_cmd[5:], expected_target_args) @unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist") + @requires_subprocess() def test_cli_module_argument_parsing(self): test_args = ["profiling.sampling.cli", "run", "-m", "mymodule"] @@ -70,8 +73,9 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("importlib.util.find_spec", return_value=True), ): - from profiling.sampling.cli import main self._setup_sync_mocks(mock_socket, mock_popen) main() @@ -80,6 +84,7 @@ class TestSampleProfilerCLI(unittest.TestCase): mock_sample.assert_called_once() @unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist") + @requires_subprocess() def test_cli_module_with_arguments(self): test_args = [ "profiling.sampling.cli", @@ -96,9 +101,10 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("importlib.util.find_spec", return_value=True), ): self._setup_sync_mocks(mock_socket, mock_popen) - from profiling.sampling.cli import main main() self._verify_coordinator_command( @@ -115,9 +121,10 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("os.path.exists", return_value=True), ): self._setup_sync_mocks(mock_socket, mock_popen) - from profiling.sampling.cli import main main() self._verify_coordinator_command(mock_popen, ("myscript.py",)) @@ -139,6 +146,8 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("os.path.exists", return_value=True), ): # Use the helper to set up mocks consistently mock_process = self._setup_sync_mocks(mock_socket, mock_popen) @@ -148,7 +157,6 @@ class TestSampleProfilerCLI(unittest.TestCase): None, ] - from profiling.sampling.cli import main main() # Verify the coordinator command was called @@ -181,7 +189,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -196,18 +203,12 @@ class TestSampleProfilerCLI(unittest.TestCase): with ( mock.patch("sys.argv", test_args), mock.patch("sys.stderr", io.StringIO()) as mock_stderr, - mock.patch("subprocess.Popen") as mock_popen, - mock.patch("socket.socket") as mock_socket, - self.assertRaises(FileNotFoundError) as cm, # Expect FileNotFoundError, not SystemExit + self.assertRaises(SystemExit) as cm, ): - self._setup_sync_mocks(mock_socket, mock_popen) - # Override to raise FileNotFoundError for non-existent script - mock_popen.side_effect = FileNotFoundError("12345") - from profiling.sampling.cli import main main() # Verify the error is about the non-existent script - self.assertIn("12345", str(cm.exception)) + self.assertIn("12345", str(cm.exception.code)) def test_cli_no_target_specified(self): # In new CLI, must specify a subcommand @@ -218,7 +219,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -226,6 +226,7 @@ class TestSampleProfilerCLI(unittest.TestCase): self.assertIn("invalid choice", error_msg) @unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist") + @requires_subprocess() def test_cli_module_with_profiler_options(self): test_args = [ "profiling.sampling.cli", @@ -248,9 +249,10 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("importlib.util.find_spec", return_value=True), ): self._setup_sync_mocks(mock_socket, mock_popen) - from profiling.sampling.cli import main main() self._verify_coordinator_command(mock_popen, ("-m", "mymodule")) @@ -278,9 +280,10 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("os.path.exists", return_value=True), ): self._setup_sync_mocks(mock_socket, mock_popen) - from profiling.sampling.cli import main main() self._verify_coordinator_command( @@ -297,7 +300,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -305,6 +307,7 @@ class TestSampleProfilerCLI(unittest.TestCase): self.assertIn("required: target", error_msg) # argparse error for missing positional arg @unittest.skipIf(is_emscripten, "socket.SO_REUSEADDR does not exist") + @requires_subprocess() def test_cli_long_module_option(self): test_args = [ "profiling.sampling.cli", @@ -319,9 +322,10 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("profiling.sampling.cli.sample") as mock_sample, mock.patch("subprocess.Popen") as mock_popen, mock.patch("socket.socket") as mock_socket, + mock.patch("profiling.sampling.cli._wait_for_ready_signal"), + mock.patch("importlib.util.find_spec", return_value=True), ): self._setup_sync_mocks(mock_socket, mock_popen) - from profiling.sampling.cli import main main() self._verify_coordinator_command( @@ -346,6 +350,7 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch( "profiling.sampling.cli._run_with_sync" ) as mock_run_with_sync, + mock.patch("os.path.exists", return_value=True), ): mock_process = mock.MagicMock() mock_process.pid = 12345 @@ -356,7 +361,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock_process.poll.return_value = None mock_run_with_sync.return_value = mock_process - from profiling.sampling.cli import main main() mock_run_with_sync.assert_called_once_with( @@ -412,8 +416,6 @@ class TestSampleProfilerCLI(unittest.TestCase): ), ] - from profiling.sampling.cli import main - for test_args, expected_error_keyword in test_cases: with ( mock.patch("sys.argv", test_args), @@ -436,7 +438,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() # Check that sample was called (exact filename depends on implementation) @@ -471,8 +472,6 @@ class TestSampleProfilerCLI(unittest.TestCase): ), ] - from profiling.sampling.cli import main - for test_args, expected_filename, expected_format in test_cases: with ( mock.patch("sys.argv", test_args), @@ -489,7 +488,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()), ): with self.assertRaises(SystemExit): - from profiling.sampling.cli import main main() def test_cli_mutually_exclusive_format_options(self): @@ -508,7 +506,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()), ): with self.assertRaises(SystemExit): - from profiling.sampling.cli import main main() def test_argument_parsing_basic(self): @@ -518,14 +515,11 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() mock_sample.assert_called_once() def test_sort_options(self): - from profiling.sampling.cli import main - sort_options = [ ("nsamples", 0), ("tottime", 1), @@ -542,7 +536,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() mock_sample.assert_called_once() @@ -556,7 +549,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() mock_sample.assert_called_once() @@ -572,7 +564,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() mock_sample.assert_called_once() @@ -587,7 +578,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.argv", test_args), mock.patch("profiling.sampling.cli.sample") as mock_sample, ): - from profiling.sampling.cli import main main() mock_sample.assert_called_once() @@ -603,7 +593,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()), self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -617,7 +606,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -633,7 +621,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -650,7 +637,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -667,7 +653,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -685,7 +670,6 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error @@ -702,10 +686,25 @@ class TestSampleProfilerCLI(unittest.TestCase): mock.patch("sys.stderr", io.StringIO()) as mock_stderr, self.assertRaises(SystemExit) as cm, ): - from profiling.sampling.cli import main main() self.assertEqual(cm.exception.code, 2) # argparse error error_msg = mock_stderr.getvalue() self.assertIn("--all-threads", error_msg) self.assertIn("incompatible with --async-aware", error_msg) + + @unittest.skipIf(is_emscripten, "subprocess not available") + def test_run_nonexistent_script_exits_cleanly(self): + """Test that running a non-existent script exits with a clean error.""" + with mock.patch("sys.argv", ["profiling.sampling.cli", "run", "/nonexistent/script.py"]): + with self.assertRaises(SystemExit) as cm: + main() + self.assertIn("Script not found", str(cm.exception.code)) + + @unittest.skipIf(is_emscripten, "subprocess not available") + def test_run_nonexistent_module_exits_cleanly(self): + """Test that running a non-existent module exits with a clean error.""" + with mock.patch("sys.argv", ["profiling.sampling.cli", "run", "-m", "nonexistent_module_xyz"]): + with self.assertRaises(SystemExit) as cm: + main() + self.assertIn("Module not found", str(cm.exception.code))