Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Linux) pid_exists() should return False if passed a TID #687

Closed
giampaolo opened this issue Sep 23, 2015 · 6 comments
Closed

(Linux) pid_exists() should return False if passed a TID #687

giampaolo opened this issue Sep 23, 2015 · 6 comments

Comments

@giampaolo
Copy link
Owner

The test case:

    def test_pid_exists_range(self):
        reap_children()
        pids = set(psutil.pids())
        for pid in range(max(pids) + 1):
            if pid not in pids:
                try:
                    assert not psutil.pid_exists(pid)
                except AssertionError:
                    assert not os.path.exists('/proc/%s' % pid)

This fails with:

Traceback (most recent call last):
  File "/home/giampaolo/svn/psutil/test/test_psutil.py", line 738, in test_pid_exists_range
    assert not os.path.exists('/proc/%s' % pid), pid
AssertionError: 947

...and happens both as root and an unprivileged user.
For some reason it seems os.listdir('/proc') won't list directory '947' but os.stat('947') won't fail (directory exists) and hence also psutil.Process(947) and psutil.pid_exists(947) succeed.
ls /proc also won't show directory 947 but ls /proc/947 succeeds.
Also ps doen't show pid 947.
This is weird.

@giampaolo
Copy link
Owner Author

This may be due to process threads: /proc/947 refers to a thread opened by another process.
That would mean that psutil.pids() is right by not showing pid 947 but psutil.pid_exists(947) returning True and psutil.Process(947) not raising NoSuchProcess are wrong.

@AkihiroSuda
Copy link

I'm also hitting this problem.
Is there any plan to support this?
Thanks.

@giampaolo
Copy link
Owner Author

So yes, it turns hidden directories not appearing in listdir('/proc') but which are still os.stat()-able refer to running Linux threads:

So the question is how to pre-emptively distinguish a process from a thread.
We need this at least for fixing psutil.pid_exists which should return False instead. It seems the easiest option is to check for pid in os.listdir('/proc'). I'm not completely happy with this because pid_exists() is supposed to provide a faster implementation than pid in psutil.pids().

Another possibility is to read /proc/pid/status: if Pid and Tgid values are different then we're dealing with a thread (and hence we should return False), but maybe this is even slower than the first solution (will have to make a benchmark).

@giampaolo
Copy link
Owner Author

It turns out checking /proc/pid/status is faster than pid in pids(). Patch:

diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index 243f162..480ad6a 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -422,7 +422,20 @@ def pids():

 def pid_exists(pid):
     """Check For the existence of a unix pid."""
-    return _psposix.pid_exists(pid)
+    exists = _psposix.pid_exists(pid)
+    if exists:
+        # 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])
+                        return tgid == pid
+                raise ValueError("'Tgid' line not found")
+        except (EnvironmentError, ValueError):
+            return pid in pids()
+    else:
+        return False


 # --- network
diff --git a/test/test_psutil.py b/test/test_psutil.py
index c2c70fe..3d334f4 100644
--- a/test/test_psutil.py
+++ b/test/test_psutil.py
@@ -744,6 +744,26 @@ class TestSystemAPIs(unittest.TestCase):
         for pid in pids:
             self.assertFalse(psutil.pid_exists(pid), msg=pid)

+    def test_pid_exists_range(self):
+        # see: https://github.com/giampaolo/psutil/issues/687
+        reap_children()
+        pids = set(psutil.pids())
+        for pid in range(max(pids) + 1):
+            if pid not in pids:
+                try:
+                    assert not psutil.pid_exists(pid)
+                except AssertionError:
+                    # race condition
+                    if pid not in psutil.pids():
+                        raise
+            else:
+                try:
+                    assert psutil.pid_exists(pid)
+                except AssertionError:
+                    # race condition
+                    if pid in psutil.pids():
+                        raise
+
     def test_pids(self):
         plist = [x.pid for x in psutil.process_iter()]
         pidlist = psutil.pids()

@tlandschoff-scale
Copy link

Got hit by this, because I am using psutil to check if the server process is still running when starting it (with the old pid coming from a pid file).

So I get this:

(python27)torsten.landschoff@horatio:~/live/service$ start_service
service still running.

Okay, what's the pid?

(python27)torsten.landschoff@horatio:~/live/service$ cat service.pid
2519
(python27)torsten.landschoff@horatio:~/live/service$ ps 2519
  PID TTY      STAT   TIME COMMAND

Err, okay, what's it doing?

(python27)torsten.landschoff@horatio:~/live/service$ python
>>> import psutil
>>> psutil.pid_exists(2519)
True
>>> 2519 in psutil.pids()
False
>>> psutil.__version__
'5.0.1'

@giampaolo giampaolo changed the title (Linux) psutil.pids() not showing all processes (Linux) pid_exists() should return False if passed a TID Jan 9, 2017
@giampaolo
Copy link
Owner Author

giampaolo commented Jan 9, 2017

OK, with 250e3a5 pid_exists() no longer returns True if passed a thread ID.
Process(tid) will still succeed though. I'm not sure I want to prevent that use case. It looks convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants