Fix python process name extraction. #11966
Conversation
Previous we grabbed the 1st 15 characters on Linux and, at least on some versions of macOS we didn't grab the process name at all, but the actual name of the process executable. Since pantsd uses process re-naming to leave a fingerprint it can use its last recorded pid has not been recycled to another process, the macOS case is problematic. In out case, the process executable for pantsd is a Python binary and processes with that as their executable are ubiquitous. This defeats the pid recycling detection scheme. Fix all this by grabbing the process name from the right place, only falling back to the prior if there is a problem reading the process command line (can happen for zombie processes at least). Add an integration test that verifies we now read the full custom command line and not a truncated version of it or the name of the process executable. [ci skip-build-wheels] [ci skip-rust]
|
This issue was discovered working #11922. |
|
|
||
| import psutil | ||
| import pytest | ||
| from _pytest.tmpdir import TempdirFactory |
There was a problem hiding this comment.
Why use Pytest rather than contextutil.temporary_dir()?
There was a problem hiding this comment.
I mean - you're the guy who doesn't like nesting. I can't win! Do you really want me to use it? Just let me know the new trumping logic if so.
There was a problem hiding this comment.
I can't win!
I'm sorry for the frustration :/
Do you really want me to use it?
The only thing that weirded me out is the private import. Otherwise, yeah, this would be great (but either approach fine)
--
It's okay to keep as is, these both work.
There was a problem hiding this comment.
I'm going to leave as is here and merge before I dash out the door. I think the pytest tmp fixutres are more useful not because of the visual reason (flattening), but the practical reason that those tmpdirs hang around under /tmp/pytest-of-jsirois/pytest-current/... so tests are easier to debug.
There was a problem hiding this comment.
Cool, that is what I was trying to get at - if you had forgotten we have .temporary_dir() vs using Pytest for a dedicated reason. Sg
I don't think so. Wrapping the pid namespace is exceedingly unlikely and this bug has been out there since 1.x days with no direct reports of issues. |
Previously we grabbed the 1st 15 characters on Linux and, at least on
some versions of macOS, we didn't grab the process name at all, but the
actual name of the process executable instead. Since pantsd uses process
re-naming to leave a fingerprint it can use to detect its last recorded
pid has not been recycled to another process, the macOS case is
problematic. In our case, the process executable for pantsd is a Python
binary and processes with that as their executable are ubiquitous. This
defeats the pid recycling detection scheme.
Fix all this by grabbing the process name from the right place, only
falling back to the prior if there is a problem reading the process
command line (can happen for zombie processes at least). Add an
integration test that verifies we now read the full custom command line
and not a truncated version of it or the name of the process executable.
[ci skip-build-wheels]
[ci skip-rust]