Skip to content

Conversation

@peytondmurray
Copy link
Contributor

@peytondmurray peytondmurray commented Sep 10, 2025

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 peytondmurray force-pushed the 732-timeout-acquire-lock branch from 3f1139e to e55e340 Compare September 30, 2025 16:28
@peytondmurray
Copy link
Contributor Author

peytondmurray commented Sep 30, 2025

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 peytondmurray changed the title [WIP] Acquire lock in separate thread Acquire lock in separate thread Sep 30, 2025
@peytondmurray peytondmurray marked this pull request as ready for review September 30, 2025 22:23
@peytondmurray peytondmurray force-pushed the 732-timeout-acquire-lock branch from a7c93d8 to 92fd140 Compare September 30, 2025 22:26
@peytondmurray
Copy link
Contributor Author

peytondmurray commented Oct 11, 2025

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.

@peytondmurray peytondmurray marked this pull request as draft October 11, 2025 20:51
@peytondmurray peytondmurray changed the title Acquire lock in separate thread [WIP] Acquire lock in separate thread Oct 11, 2025
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.

Multiprocessing Deadlock with v0.4.0

1 participant