Acquire lock in separate thread

[peytondmurray]

This PR modifies the way that the process to be inspected gets locked.

Before, py-spy would first check the process for active threads before locking the process (otherwise all threads are reported as being idle). If the process exited before a lock could be acquired, py-spy would hang while waiting for the status of the PID to change.

With this PR, acquiring the lock on the process happens in a separate thread; if the thread doesn't acquire a lock in a specified time (default is 1s, but this is a config variable. If this is too short we can increase it) we simply error out instead of hanging. Closes #732.

I've also fixed an issue with installing the numpy>=2 optional test requirement in the test workflow.

[peytondmurray]

Okay, this ended up being trickier than I thought, because not only do you need a locker thread responsible for locking the target process, but you also need to keep the remoteprocess::Lock that gets acquired in that thread alive until samples are taken. If you don't do this, the remoteprocess::Lock will unpause the target thread when it gets dropped as the locker thread exits, screwing up the sampling.

So my original solution in which the locker thread finishes execution immediately, it would just instantly unpause the target thread before samples could be taken. Happily this is something we're testing for with test_recursive, so I was able to catch it.

Please let me know if you have any comments or if you feel this could be handled more gracefully, I'd be grateful for any feedback.

[peytondmurray]

Ack, I pushed the last commit after testing (and passing locally on linux), and then was away for the past couple of weeks. I see now the Windows tests are failing, but I don't know why. I'll try spinning up a VM to test this out, but any help here would be appreciated. Moving back into draft until this gets resolved.

[Carreau]

I pushed an empty commit to re-trigger CI, but workflow needs approval again; as far as I can tell the failing test now pass on my fork I think (https://github.com/Carreau/py-spy/actions/runs/21870162508/job/63122773473)

[peytondmurray]

☝️ Thanks for double checking about this. Let me mark this as ready for review.

[peytondmurray]

Hey all, it would be great to see this merged at some point - would it be possible to retrigger CI?

[pezosanta]

Hello @peytondmurray !

When can we expect this feature to be merged and released? :)

[peytondmurray]

Hey @pezosanta, thanks for the tag: looks like I need to resolve these conflicts. I'll take care of that today, but I don't have the power to approve workflows or merge these changes.

[peytondmurray]

Hi all,

In resolving the conflicts with the current master I realized that test_busy_loop would intermittently fail with my previous implementation. This happened because the TestRunner struct used in that test would attempt to get stack traces from a busylooping python thread, sometimes multiple times in a row.

Previously with the locking mechanism that I implemented above, the separate locker thread would only block the main thread when the main thread requested the target process to be locked, but not when it was released. When the TestRunner tried to get stack traces from the busylooping python thread multiple times (e.g. while the interpreter started up), it would in some cases try to acquire a lock on the thread before the previous call's locker thread had released it.

Now I've implemented an additional check: the main thread will now block until it receives an acknowledgement that the locker thread has released the lock on the target python thread. That seems to have resolved the issue, and I've documented how it works in a diagram in PythonSpy::lock_process_with_timeout.

I'd appreciate a second look if anyone has time. And please let me know if there's anything I can do to get this across the finish line!

[pezosanta]

Hi @peytondmurray !

Any updates so far? 🙃

[peytondmurray]

No updates from me, I'm waiting on the maintainers and community for review and merge. If there's some feedback that anyone (maybe you?) has for me, I'm happy to make whatever changes are needed to get this merged.

[pezosanta]

@peytondmurray I'm sorry, but I don't code in Rust :)

However, I'm wondering why is it this hard to get a review from a contributor... The last release was 5 month ago, the last commit to master was 4 month ago... Do they just stop developing / caring about this tool or... ?

[peytondmurray]

Maintaining open source code is largely something people do for free when they can. People have other obligations, other interests, and only finite time and energy. Most successful open source projects have healthy community input, shared responsibility, and maintainer redundancy, but not all projects are able to reach that point. I have no expectations of OSS maintainers, and am just happy if folks are able and interested and willing to review any PRs I make 🙂

I'm sorry, but I don't code in Rust :)

Given that this PR has been open for a year already, and that the activity on this project is largely just maintenance a few times a year, I'd suggest learning Rust! In the time it takes to do that, you'll either be able to help out with maintainer/community duties here or you'll be able to start your own python profiler project. Then you can maintain it as frequently as you'd like :)

That said my preference would be to avoid splitting community effort between similar projects. We're stronger if we work together. So for any maintainer reading this, I'd be happy to exchange PR review duties if it would help reduce the burden of reviewing this one - just say the word.