fixup! Merge branch 'main' into tachyon-file-does-not-exist

Author: pablogsal

Lib/profiling/sampling/_sync_coordinator.py

@@ -229,7 +229,9 @@ def main() -> NoReturn:
         else:
             script_path = target_args[0]
             script_args = target_args[1:]
-            if not os.path.isfile(os.path.join(cwd, script_path)):
+            # 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

Lib/profiling/sampling/cli.py

@@ -2,7 +2,7 @@
 
 import argparse
 import os
-import select
+import selectors
 import socket
 import subprocess
 import sys
@@ -94,21 +94,64 @@ 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
     with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sync_sock:
-        # Set SO_REUSEADDR to avoid "Address already in use" errors
         sync_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-        sync_sock.bind(("127.0.0.1", 0))  # Let OS choose a free port
+        sync_sock.bind(("127.0.0.1", 0))
         sync_port = sync_sock.getsockname()[1]
         sync_sock.listen(1)
         sync_sock.settimeout(_SYNC_TIMEOUT)
 
-        # Get current working directory to preserve it
         cwd = os.getcwd()
-
-        # Build command using the sync coordinator
         target_args = original_cmd[1:]  # Remove python executable
         cmd = (
             sys.executable,
@@ -118,50 +161,18 @@ def _run_with_sync(original_cmd, suppress_output=False):
             cwd,
         ) + tuple(target_args)
 
-        # Start the process with coordinator
-        # Suppress stdout if requested (for live mode), but capture stderr
-        # so we can report errors if the process fails to start
-        popen_kwargs = {}
+        popen_kwargs = {"stderr": subprocess.PIPE}
         if suppress_output:
             popen_kwargs["stdin"] = subprocess.DEVNULL
             popen_kwargs["stdout"] = subprocess.DEVNULL
-            popen_kwargs["stderr"] = subprocess.PIPE
 
         process = subprocess.Popen(cmd, **popen_kwargs)
 
         try:
-            # Wait for ready signal, but also check if process dies early
-            deadline = time.monotonic() + _SYNC_TIMEOUT
-            while True:
-                # Check if process died
-                if process.poll() is not None:
-                    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}"
-                    )
-
-                # Check remaining timeout
-                remaining = deadline - time.monotonic()
-                if remaining <= 0:
-                    raise socket.timeout("timed out")
-
-                # Wait for socket with short timeout to allow process checks
-                ready, _, _ = select.select([sync_sock], [], [], min(0.1, remaining))
-                if ready:
-                    with sync_sock.accept()[0] as conn:
-                        ready_signal = conn.recv(_RECV_BUFFER_SIZE)
-                        if ready_signal != _READY_MESSAGE:
-                            raise RuntimeError(
-                                f"Invalid ready signal received: {ready_signal!r}"
-                            )
-                        break
+            _wait_for_ready_signal(sync_sock, process, _SYNC_TIMEOUT)
+
+            if process.stderr:
+                process.stderr.close()
 
         except socket.timeout:
             # If we timeout, kill the process and raise an error
@@ -670,8 +681,10 @@ def _handle_run(args):
     else:
         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 = (
@@ -718,14 +731,6 @@ def _handle_run(args):
 
 def _handle_live_attach(args, pid):
     """Handle live mode for an existing process."""
-    # Check if process exists
-    try:
-        os.kill(pid, 0)
-    except ProcessLookupError:
-        sys.exit(f"Error: process not found: {pid}")
-    except PermissionError:
-        pass  # Process exists, permission error will be handled later
-
     mode = _parse_mode(args.mode)
 
     # Determine skip_idle based on mode
@@ -766,9 +771,10 @@ def _handle_live_run(args):
     else:
         cmd = (sys.executable, args.target, *args.args)
 
-    # Run with synchronization, suppressing output for live mode
-    # Note: _run_with_sync will raise if the process dies before signaling ready
-    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)
 

Lib/test/test_profiling/test_sampling_profiler/test_cli.py

@@ -70,7 +70,7 @@ def test_cli_module_argument_parsing(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             from profiling.sampling.cli import main
             self._setup_sync_mocks(mock_socket, mock_popen)
@@ -97,7 +97,7 @@ def test_cli_module_with_arguments(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             self._setup_sync_mocks(mock_socket, mock_popen)
             from profiling.sampling.cli import main
@@ -117,7 +117,7 @@ def test_cli_script_argument_parsing(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             self._setup_sync_mocks(mock_socket, mock_popen)
             from profiling.sampling.cli import main
@@ -142,7 +142,7 @@ def test_cli_script_with_arguments(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             # Use the helper to set up mocks consistently
             mock_process = self._setup_sync_mocks(mock_socket, mock_popen)
@@ -252,7 +252,7 @@ def test_cli_module_with_profiler_options(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             self._setup_sync_mocks(mock_socket, mock_popen)
             from profiling.sampling.cli import main
@@ -283,7 +283,7 @@ def test_cli_script_with_profiler_options(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             self._setup_sync_mocks(mock_socket, mock_popen)
             from profiling.sampling.cli import main
@@ -325,7 +325,7 @@ def test_cli_long_module_option(self):
             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.select.select", return_value=([True], [], [])),
+            mock.patch("profiling.sampling.cli._wait_for_ready_signal"),
         ):
             self._setup_sync_mocks(mock_socket, mock_popen)
             from profiling.sampling.cli import main
@@ -716,3 +716,21 @@ def test_async_aware_incompatible_with_all_threads(self):
         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."""
+        from profiling.sampling.cli import main
+        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."""
+        from profiling.sampling.cli import main
+        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))