GH-142591: Tachyon does not handle non-existent file/module by savannahostrowski · Pull Request #142592 · python/cpython · GitHub
Skip to content

GH-142591: Tachyon does not handle non-existent file/module#142592

Merged
pablogsal merged 6 commits into
python:mainfrom
savannahostrowski:tachyon-file-does-not-exist
Dec 14, 2025
Merged

GH-142591: Tachyon does not handle non-existent file/module#142592
pablogsal merged 6 commits into
python:mainfrom
savannahostrowski:tachyon-file-does-not-exist

Conversation

@savannahostrowski

@savannahostrowski savannahostrowski commented Dec 11, 2025

Copy link
Copy Markdown
Member

This validates script/module/PID exists before starting the profiler. If the target doesn't exist, we now fail fast with a clear error instead of launching an empty TUI.

Screenshot 2025-12-11 at 11 00 21 AM Screenshot 2025-12-11 at 11 00 09 AM

@savannahostrowski

Copy link
Copy Markdown
Member Author

Comment thread Lib/profiling/sampling/_sync_coordinator.py
Comment thread Lib/profiling/sampling/cli.py Outdated
"""Handle live mode for an existing process."""
# Check if process exists
try:
os.kill(pid, 0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this works on windows. On Windows, os.kill() only supports signals SIGTERM, CTRL_C_EVENT, and CTRL_BREAK_EVENT. Signal 0 (which is the Unix idiom for checking if a process exists without sending a signal) is not supported on Windows.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also has a TOCTOU problem: the process may die after you check and before we attach

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix probably needs to be done more on the line of https://github.com/python/cpython/pull/142655/changes (we should handle this there)

@pablogsal

pablogsal commented Dec 14, 2025

Copy link
Copy Markdown
Member

@pablogsal pablogsal force-pushed the tachyon-file-does-not-exist branch 3 times, most recently from fe664f9 to 980b035 Compare December 14, 2025 03:16

@pablogsal pablogsal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks a lot for the PR @savannahostrowski ❤️

@pablogsal pablogsal merged commit f893e8f into python:main Dec 14, 2025
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants