fix Linux daemon PID lookup without lsof - #3523
Conversation
ebarkhordar
left a comment
There was a problem hiding this comment.
Ran this against a real listener rather than the mocked subprocess.run, since CI's test-embed job is skipped on this PR (it is gated on has_secrets, which is false for a fork), so the new test has not executed upstream.
python:3.12-slim plus iproute2, lsof absent, a real socket bound to 127.0.0.1:9177, the package pip-installed from each side:
- base
205e47b4:_find_pid_on_port(9177)returnsNone - head
b69873f3: returns 342, which is the listener's own pid tests/test_embed_manager.pyat head: 22 passed
So the ss -H -ltnp "sport = :9177" argv is valid as written (iproute2 6.15.0), which the mocked test cannot show on its own.
One gap, not blocking. #3517 lists minimal containers next to CachyOS, and those often have neither binary. On stock python:3.12-slim (no lsof, no ss) head still returns None, and since #3171 a missing pid makes stop() return False rather than only log a warning. A stdlib fallback would cover that case with no new dependency: read /proc/net/tcp and /proc/net/tcp6 for the listening socket's inode (state 0A), then match it against the /proc/[0-9]*/fd/* symlinks. I tried that in a container with neither binary and it resolved the correct pid. Fine as a follow-up if you would rather keep this PR to the environment that was reported.
|
Added a stdlib |
80d75f5 to
6424947
Compare
Complements #3585 for minimal Linux images where neither
lsofnorssis installed.PID discovery now falls back to procfs: it finds listening TCP/TCP6 socket inodes for the target port in
/proc/net, then resolves the owning process through/proc/<pid>/fdsymlinks. Existinglsofandsspaths remain preferred.Validation after rebasing onto current
main:uv run pytest tests/test_embed_manager.py -q— 28 passedenv -u HTTP_PROXY -u HTTPS_PROXY -u ALL_PROXY -u http_proxy -u https_proxy -u all_proxy NO_PROXY=127.0.0.1,localhost uv run pytest tests/ -q— 187 passeduv run ruff check hindsight_embed tests— passeduv run ruff format --check hindsight_embed tests— 22 files already formattedgit diff --check— passedThe proxy variables are removed for the full suite because the local environment points HTTPX at a SOCKS proxy without its optional
socksiodependency; the first run otherwise produced 18 control-center transport setup failures unrelated to this patch.