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

Resolve race condition in Process.threads() #2151

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

li-dan
Copy link
Contributor

@li-dan li-dan commented Sep 29, 2022

Process.threads() has a race condition triggered when a thread exits after the open_binary() call and before the read() call. When this happens, the read() call raises ProcessLookupError.

Handle the race condition by catching ProcessLookupError from read() and treating the same as a FileNotFoundError from open_binary(). This is the same approach used in ppid_map().

Signed-off-by: Daniel Li daniel.li@deshaw.com

Summary

Description

Fix the race condition in Process.threads() using the same approach as ppid_map().

Process.threads() has a race condition triggered when a thread exits
after the open_binary() call and before the read() call. When this
happens, the read() call raises ProcessLookupError.

Handle the race condition by catching ProcessLookupError from read() and
treating the same as a FileNotFoundError from open_binary(). This is the
same approach used in ppid_map().

Signed-off-by: Daniel Li <daniel.li@deshaw.com>
@li-dan li-dan force-pushed the fix_process_threads_race_condition branch from 3eaba00 to 4edc620 Compare September 29, 2022 01:31
@giampaolo
Copy link
Owner

I see there is the same problem in open_files() method. While you're at it, could you please fix that one as well?
Thanks a lot

@li-dan
Copy link
Contributor Author

li-dan commented Sep 29, 2022

Are you referring to these lines?

psutil/psutil/_pslinux.py

Lines 2213 to 2220 in 69b572e

# Get file position and flags.
file = "%s/%s/fdinfo/%s" % (
self._procfs_path, self.pid, fd)
try:
with open_binary(file) as f:
pos = int(f.readline().split()[1])
flags = int(f.readline().split()[1], 8)
except FileNotFoundError:

If the file descriptor disappears between the open_binary() and readline() calls, then readline() raises FileNotFoundError. So I believe that except FileNotFoundError is correct in open_files().

@giampaolo
Copy link
Owner

Yes I mean those lines. How do you know readline() can only raise FileNotFoundError? I think calling readline() should be like calling read().

Signed-off-by: Daniel Li <daniel.li@deshaw.com>
@li-dan
Copy link
Contributor Author

li-dan commented Sep 29, 2022

In my testing, it seems that .../fdinfo/... behaves differently from .../task/.... For example:

f1 = open("/etc/motd")
with open(f"/proc/{os.getpid()}/fdinfo/{f1.fileno()}", "rb") as f2:
    f1.close()
    f2.readline() # Raises FileNotFoundError

It's possible that some other Linux kernel will do something different, and it won't hurt either way, so I changed open_files() too.

@giampaolo
Copy link
Owner

Yeah let's just add it to be sure. Thanks for your PR.

@giampaolo giampaolo merged commit 052c1e2 into giampaolo:master Sep 29, 2022
@li-dan li-dan deleted the fix_process_threads_race_condition branch September 29, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Linux] Race condition in Process.threads()
2 participants