Skip to content

Do not fail sampling completely if a single thread dies before sampling - #771

Merged
benfred merged 2 commits into
benfred:masterfrom
I-Al-Istannen:fix/do-not-give-up-on-process
Jul 13, 2025
Merged

Do not fail sampling completely if a single thread dies before sampling#771
benfred merged 2 commits into
benfred:masterfrom
I-Al-Istannen:fix/do-not-give-up-on-process

Conversation

@I-Al-Istannen

Copy link
Copy Markdown
Contributor

The flow when sampling a process starts by

  1. Enumerate all threads by reading /proc/pid/task/<tid>.
  2. For each found thread, check if it is currently active by reading its state file in /proc/<tid>/stat.

During this time, we did not suspend the process yet. Therefore, it might happen that a thread dies precisely in the milliseconds between the enumeration readdir and the stat for active checking. In those cases, py-spy bails out completely, skipping all threads. If a process creates a lot of threads and tears them down (e.g. through using some native dependency with a saner threading model, such as rust with polars), this case can be hit a lot.

I do not see why we would want to throw out a full sample, just because a single thread died prematurely. Therefore, I propose silently ignoring stat errors for threads. The .threads() method will re-enumerate and find the thread lacking, if it is called in later parts of the code.

Doing this also exposed benfred/remoteprocess#106.

Even without this patch, sampling might fail with EPERM, but that is an expected quirk of ptrace: https://elixir.bootlin.com/linux/v6.15.3/source/kernel/ptrace.c#L458 if the process is currently exiting, ptrace returns EPERM instead of ESRCH.

@I-Al-Istannen

I-Al-Istannen commented Jul 12, 2025

Copy link
Copy Markdown
Contributor Author

I have a stress test lying around with:

import threading

import os

print(os.getpid())

def _dummy():
    #time.sleep(0.1)
    pass

for index in range(0, 100_000):
    if index % 1000 == 0:
        print("Iteration: ", index)
    ts = []
    for i in range(0, 10):
        t = threading.Thread(target=_dummy)
        ts.append(t)
        t.start()
    for t in ts:
        t.join()

print("HEY")

If you record this with py-spy top --pid <pid> it will have a quite high error rate of ~50% in my tests, without this patch.

ETA: With this patch and my remoteprocess change, it is ~1%.

@benfred
benfred merged commit ee6a780 into benfred:master Jul 13, 2025
74 checks passed
@I-Al-Istannen
I-Al-Istannen deleted the fix/do-not-give-up-on-process branch July 13, 2025 12:52
@benfred benfred added the bug Something isn't working label Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants