From 250e3a51c13840501e01a7000b476742a220ea46 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Mon, 9 Jan 2017 16:49:27 +0100 Subject: [PATCH] #687: [Linux] pid_exists() no longer returns True if passed a process thread ID --- HISTORY.rst | 4 ++++ docs/index.rst | 8 +++++--- psutil/_pslinux.py | 28 ++++++++++++++++++++++++++-- psutil/tests/test_linux.py | 19 +++++++++++++++++++ psutil/tests/test_process.py | 10 ++-------- 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 94feab8e3..92413ec1e 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,8 +3,12 @@ 5.0.2 ===== +*XXXX-XX-XX* + **Bug fixes** +- 687_: [Linux] pid_exists() no longer returns True if passed a process thread + ID. - 948_: cannot install psutil with PYTHONOPTIMIZE=2. diff --git a/docs/index.rst b/docs/index.rst index 3aed80943..022c6a6ba 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -738,10 +738,12 @@ Process class .. class:: Process(pid=None) - Represents an OS process with the given *pid*. If *pid* is omitted current - process *pid* (`os.getpid() `__) - is used. + Represents an OS process with the given *pid*. + If *pid* is omitted current process *pid* + (`os.getpid() `__) is used. Raise :class:`NoSuchProcess` if *pid* does not exist. + On Linux *pid* can also refer to a thread ID (the *id* field returned by + :meth:`threads` method). When accessing methods of this class always be prepared to catch :class:`NoSuchProcess`, :class:`ZombieProcess` and :class:`AccessDenied` exceptions. diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index 91fdae4f8..fe2f459dd 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -1070,8 +1070,32 @@ def pids(): def pid_exists(pid): - """Check For the existence of a unix pid.""" - return _psposix.pid_exists(pid) + """Check for the existence of a unix PID.""" + if not _psposix.pid_exists(pid): + return False + else: + # Linux's apparently does not distinguish between PIDs and TIDs + # (thread IDs). + # listdir("/proc") won't show any TID (only PIDs) but + # os.stat("/proc/{tid}") will succeed if {tid} exists. + # os.kill() can also be passed a TID. This is quite confusing. + # In here we want to enforce this distinction and support PIDs + # only, see: + # https://github.com/giampaolo/psutil/issues/687 + try: + # Note: already checked that this is faster than using a + # regular expr. Also (a lot) faster than doing + # 'return pid in pids()' + with open_binary("%s/%s/status" % (get_procfs_path(), pid)) as f: + for line in f: + if line.startswith(b"Tgid:"): + tgid = int(line.split()[1]) + # If tgid and pid are the same then we're + # dealing with a process PID. + return tgid == pid + raise ValueError("'Tgid' line not found") + except (EnvironmentError, ValueError): + return pid in pids() def wrap_exceptions(fun): diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py index 871cb74eb..028a41d9c 100755 --- a/psutil/tests/test_linux.py +++ b/psutil/tests/test_linux.py @@ -40,6 +40,7 @@ from psutil.tests import sh from psutil.tests import skip_on_not_implemented from psutil.tests import TESTFN +from psutil.tests import ThreadTask from psutil.tests import TRAVIS from psutil.tests import unittest from psutil.tests import which @@ -1004,6 +1005,24 @@ def open_mock(name, *args, **kwargs): importlib.reload(psutil._pslinux) importlib.reload(psutil) + def test_issue_687(self): + # In case of thread ID: + # - pid_exists() is supposed to return False + # - Process(tid) is supposed to work + # - pids() should not return the TID + # See: https://github.com/giampaolo/psutil/issues/687 + t = ThreadTask() + t.start() + try: + p = psutil.Process() + tid = p.threads()[1].id + assert not psutil.pid_exists(tid), tid + pt = psutil.Process(tid) + pt.as_dict() + self.assertNotIn(tid, psutil.pids()) + finally: + t.stop() + # ===================================================================== # test process diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py index 50e0cc746..d25f44749 100755 --- a/psutil/tests/test_process.py +++ b/psutil/tests/test_process.py @@ -501,10 +501,8 @@ def test_num_threads(self): try: step2 = p.num_threads() self.assertEqual(step2, step1 + 1) - thread.stop() finally: - if thread._running: - thread.stop() + thread.stop() @unittest.skipUnless(WINDOWS, 'WINDOWS only') def test_num_handles(self): @@ -524,7 +522,6 @@ def test_threads(self): thread = ThreadTask() thread.start() - try: step2 = p.threads() self.assertEqual(len(step2), len(step1) + 1) @@ -536,11 +533,8 @@ def test_threads(self): self.assertEqual(athread.id, athread[0]) self.assertEqual(athread.user_time, athread[1]) self.assertEqual(athread.system_time, athread[2]) - # test num threads - thread.stop() finally: - if thread._running: - thread.stop() + thread.stop() @retry_before_failing() # see: https://travis-ci.org/giampaolo/psutil/jobs/111842553