Skip to content

Commit

Permalink
Fix python process name extraction. (pantsbuild#11966)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jsirois authored Apr 23, 2021
1 parent 8df69e3 commit bc4753c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
11 changes: 9 additions & 2 deletions src/python/pants/pantsd/process_manager.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import logging
import os
import signal
Expand Down Expand Up @@ -331,9 +333,14 @@ def write_pid(self, pid: Optional[int] = None):
pid = os.getpid() if pid is None else pid
self.write_metadata_by_name(self.PID_KEY, str(pid))

def _get_process_name(self, process: psutil.Process | None = None) -> str:
proc = process or self._as_process()
cmdline = proc.cmdline()
return cast(str, cmdline[0] if cmdline else proc.name())

def write_process_name(self, process_name: Optional[str] = None):
"""Write the current process's name."""
process_name = process_name or self._as_process().name()
process_name = process_name or self._get_process_name()
self.write_metadata_by_name(self.PROCESS_NAME_KEY, process_name)

def write_socket(self, socket_info: int):
Expand Down Expand Up @@ -382,7 +389,7 @@ def is_alive(self, extended_check=None):
(process.status() == psutil.STATUS_ZOMBIE)
or
# Check for stale pids.
(self.process_name and self.process_name != process.name())
(self.process_name and self.process_name != self._get_process_name(process))
or
# Extended checking.
(extended_check and not extended_check(process))
Expand Down
36 changes: 35 additions & 1 deletion tests/python/pants_test/pantsd/test_process_manager.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import errno
import logging
import os
import subprocess
import sys
import unittest
import unittest.mock
from contextlib import contextmanager
from textwrap import dedent

import psutil
import pytest
from _pytest.tmpdir import TempdirFactory

from pants.pantsd.process_manager import ProcessManager
from pants.util.contextutil import temporary_dir
Expand Down Expand Up @@ -334,3 +337,34 @@ def test_callbacks(self):
self.pm.pre_fork()
self.pm.post_fork_child()
self.pm.post_fork_parent()


def test_process_name_setproctitle_integration(tmpdir_factory: TempdirFactory) -> None:
buildroot = tmpdir_factory.mktemp("buildroot")
manager_name = "Bob"
process_name = f"{manager_name} [{buildroot}]"
metadata_base_dir = tmpdir_factory.mktemp(".pids")

subprocess.check_call(
args=[
sys.executable,
"-c",
dedent(
f"""\
from setproctitle import setproctitle as set_process_title
from pants.pantsd.process_manager import ProcessManager
set_process_title({process_name!r})
pm = ProcessManager({manager_name!r}, metadata_base_dir="{metadata_base_dir}")
pm.write_pid()
pm.write_process_name()
"""
),
],
env=dict(PYTHONPATH=os.pathsep.join(sys.path)),
)

pm = ProcessManager(manager_name, metadata_base_dir=str(metadata_base_dir))
assert process_name == pm.process_name

0 comments on commit bc4753c

Please sign in to comment.