Skip to content

Commit 2ee9bf9

Browse files
Fail fast
1 parent 00ef823 commit 2ee9bf9

File tree

3 files changed

+69
-27
lines changed

3 files changed

+69
-27
lines changed

Lib/profiling/sampling/_sync_coordinator.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def _execute_module(module_name: str, module_args: List[str]) -> None:
138138
"""
139139
# Replace sys.argv to match how Python normally runs modules
140140
# When running 'python -m module args', sys.argv is ["__main__.py", "args"]
141-
sys.argv = [f"__main__.py"] + module_args
141+
sys.argv = ["__main__.py"] + module_args
142142

143143
try:
144144
runpy.run_module(module_name, run_name="__main__", alter_sys=True)
@@ -215,22 +215,30 @@ def main() -> NoReturn:
215215
# Set up execution environment
216216
_setup_environment(cwd)
217217

218-
# Signal readiness to profiler
219-
_signal_readiness(sync_port)
220-
221-
# Execute the target
222-
if target_args[0] == "-m":
223-
# Module execution
218+
# Determine execution type and validate target exists
219+
is_module = target_args[0] == "-m"
220+
if is_module:
224221
if len(target_args) < 2:
225222
raise ArgumentError("Module name required after -m")
226-
227223
module_name = target_args[1]
228224
module_args = target_args[2:]
229-
_execute_module(module_name, module_args)
225+
226+
import importlib.util
227+
if importlib.util.find_spec(module_name) is None:
228+
raise TargetError(f"Module not found: {module_name}")
230229
else:
231-
# Script execution
232230
script_path = target_args[0]
233231
script_args = target_args[1:]
232+
if not os.path.isfile(os.path.join(cwd, script_path)):
233+
raise TargetError(f"Script not found: {script_path}")
234+
235+
# Signal readiness to profiler
236+
_signal_readiness(sync_port)
237+
238+
# Execute the target
239+
if is_module:
240+
_execute_module(module_name, module_args)
241+
else:
234242
_execute_script(script_path, script_args, cwd)
235243

236244
except CoordinatorError as e:

Lib/profiling/sampling/cli.py

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
import argparse
44
import os
5+
import select
56
import socket
67
import subprocess
78
import sys
9+
import time
810

911
from .sample import sample, sample_live
1012
from .pstats_collector import PstatsCollector
@@ -119,25 +121,50 @@ def _run_with_sync(original_cmd, suppress_output=False):
119121
) + tuple(target_args)
120122

121123
# Start the process with coordinator
122-
# Suppress stdout/stderr if requested (for live mode)
124+
# Suppress stdout if requested (for live mode), but capture stderr
125+
# so we can report errors if the process fails to start
123126
popen_kwargs = {}
124127
if suppress_output:
125128
popen_kwargs["stdin"] = subprocess.DEVNULL
126129
popen_kwargs["stdout"] = subprocess.DEVNULL
127-
popen_kwargs["stderr"] = subprocess.DEVNULL
130+
popen_kwargs["stderr"] = subprocess.PIPE
128131

129132
process = subprocess.Popen(cmd, **popen_kwargs)
130133

131134
try:
132-
# Wait for ready signal with timeout
133-
with sync_sock.accept()[0] as conn:
134-
ready_signal = conn.recv(_RECV_BUFFER_SIZE)
135-
136-
if ready_signal != _READY_MESSAGE:
135+
# Wait for ready signal, but also check if process dies early
136+
deadline = time.monotonic() + _SYNC_TIMEOUT
137+
while True:
138+
# Check if process died
139+
if process.poll() is not None:
140+
stderr_msg = ""
141+
if process.stderr:
142+
try:
143+
stderr_msg = process.stderr.read().decode().strip()
144+
except (OSError, UnicodeDecodeError):
145+
pass
146+
if stderr_msg:
147+
raise RuntimeError(stderr_msg)
137148
raise RuntimeError(
138-
f"Invalid ready signal received: {ready_signal!r}"
149+
f"Process exited with code {process.returncode}"
139150
)
140151

152+
# Check remaining timeout
153+
remaining = deadline - time.monotonic()
154+
if remaining <= 0:
155+
raise socket.timeout("timed out")
156+
157+
# Wait for socket with short timeout to allow process checks
158+
ready, _, _ = select.select([sync_sock], [], [], min(0.1, remaining))
159+
if ready:
160+
with sync_sock.accept()[0] as conn:
161+
ready_signal = conn.recv(_RECV_BUFFER_SIZE)
162+
if ready_signal != _READY_MESSAGE:
163+
raise RuntimeError(
164+
f"Invalid ready signal received: {ready_signal!r}"
165+
)
166+
break
167+
141168
except socket.timeout:
142169
# If we timeout, kill the process and raise an error
143170
if process.poll() is None:
@@ -614,15 +641,6 @@ def _handle_attach(args):
614641

615642
def _handle_run(args):
616643
"""Handle the 'run' command."""
617-
# Validate target exists before launching
618-
if args.module:
619-
import importlib.util
620-
if importlib.util.find_spec(args.target) is None:
621-
sys.exit(f"Error: module not found: {args.target}")
622-
else:
623-
if not os.path.exists(args.target):
624-
sys.exit(f"Error: script not found: {args.target}")
625-
626644
# Check if live mode is requested
627645
if args.live:
628646
_handle_live_run(args)
@@ -681,6 +699,14 @@ def _handle_run(args):
681699

682700
def _handle_live_attach(args, pid):
683701
"""Handle live mode for an existing process."""
702+
# Check if process exists
703+
try:
704+
os.kill(pid, 0)
705+
except ProcessLookupError:
706+
sys.exit(f"Error: process not found: {pid}")
707+
except PermissionError:
708+
pass # Process exists, permission error will be handled later
709+
684710
mode = _parse_mode(args.mode)
685711

686712
# Determine skip_idle based on mode
@@ -720,6 +746,7 @@ def _handle_live_run(args):
720746
cmd = (sys.executable, args.target, *args.args)
721747

722748
# Run with synchronization, suppressing output for live mode
749+
# Note: _run_with_sync will raise if the process dies before signaling ready
723750
process = _run_with_sync(cmd, suppress_output=True)
724751

725752
mode = _parse_mode(args.mode)

Lib/test/test_profiling/test_sampling_profiler/test_cli.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ def test_cli_module_argument_parsing(self):
7070
mock.patch("profiling.sampling.cli.sample") as mock_sample,
7171
mock.patch("subprocess.Popen") as mock_popen,
7272
mock.patch("socket.socket") as mock_socket,
73+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
7374
):
7475
from profiling.sampling.cli import main
7576
self._setup_sync_mocks(mock_socket, mock_popen)
@@ -96,6 +97,7 @@ def test_cli_module_with_arguments(self):
9697
mock.patch("profiling.sampling.cli.sample") as mock_sample,
9798
mock.patch("subprocess.Popen") as mock_popen,
9899
mock.patch("socket.socket") as mock_socket,
100+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
99101
):
100102
self._setup_sync_mocks(mock_socket, mock_popen)
101103
from profiling.sampling.cli import main
@@ -115,6 +117,7 @@ def test_cli_script_argument_parsing(self):
115117
mock.patch("profiling.sampling.cli.sample") as mock_sample,
116118
mock.patch("subprocess.Popen") as mock_popen,
117119
mock.patch("socket.socket") as mock_socket,
120+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
118121
):
119122
self._setup_sync_mocks(mock_socket, mock_popen)
120123
from profiling.sampling.cli import main
@@ -139,6 +142,7 @@ def test_cli_script_with_arguments(self):
139142
mock.patch("profiling.sampling.cli.sample") as mock_sample,
140143
mock.patch("subprocess.Popen") as mock_popen,
141144
mock.patch("socket.socket") as mock_socket,
145+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
142146
):
143147
# Use the helper to set up mocks consistently
144148
mock_process = self._setup_sync_mocks(mock_socket, mock_popen)
@@ -248,6 +252,7 @@ def test_cli_module_with_profiler_options(self):
248252
mock.patch("profiling.sampling.cli.sample") as mock_sample,
249253
mock.patch("subprocess.Popen") as mock_popen,
250254
mock.patch("socket.socket") as mock_socket,
255+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
251256
):
252257
self._setup_sync_mocks(mock_socket, mock_popen)
253258
from profiling.sampling.cli import main
@@ -278,6 +283,7 @@ def test_cli_script_with_profiler_options(self):
278283
mock.patch("profiling.sampling.cli.sample") as mock_sample,
279284
mock.patch("subprocess.Popen") as mock_popen,
280285
mock.patch("socket.socket") as mock_socket,
286+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
281287
):
282288
self._setup_sync_mocks(mock_socket, mock_popen)
283289
from profiling.sampling.cli import main
@@ -319,6 +325,7 @@ def test_cli_long_module_option(self):
319325
mock.patch("profiling.sampling.cli.sample") as mock_sample,
320326
mock.patch("subprocess.Popen") as mock_popen,
321327
mock.patch("socket.socket") as mock_socket,
328+
mock.patch("profiling.sampling.cli.select.select", return_value=([True], [], [])),
322329
):
323330
self._setup_sync_mocks(mock_socket, mock_popen)
324331
from profiling.sampling.cli import main

0 commit comments

Comments
 (0)