-
Notifications
You must be signed in to change notification settings - Fork 53
Fix handling of duplicate _PyRuntime symbols when libraries dlopen Python #258
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
==========================================
+ Coverage 82.27% 82.30% +0.03%
==========================================
Files 46 48 +2
Lines 6334 6353 +19
Branches 470 473 +3
==========================================
+ Hits 5211 5229 +18
- Misses 1123 1124 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7531932
to
2c1f8bb
Compare
Previously the test suite would hang instead of failing, with no indication of what had gone wrong. Signed-off-by: Matt Wozniski <[email protected]>
`uv` uses the `python-standalone` project for its interpreters, which use an unusual and not fully supported build workflow. Add a test to our CI to ensure that pytest works properly with uv's interpreters. Signed-off-by: Matt Wozniski <[email protected]>
We were incorrectly printing that a 3.13.7 process was 3.13.70 because we would always print the serial number (always 0 for a final release) whenever the release level was not a null string - and for final releases, we had it set to the empty string. Fix this to check for empty strings, instead, and ensure we never set it to a null string, instead using a recognizable sentinel for invalid release levels. Signed-off-by: Matt Wozniski <[email protected]>
When libraries like `llvmlite` `dlopen` the Python binary, it creates duplicate mappings of the binary in the process memory. This caused PyStack to use the wrong address for `_PyRuntime`, leading to "Invalid address in remote process" errors. It happens to work if we stop searching as soon as we find the symbol, rather than continuing to iterate over other loaded modules searching for the symbol. This counts on the earliest mapping for the interpreter binary being the initial mapping made by the loader. Signed-off-by: Pablo Galindo Salgado <[email protected]> Signed-off-by: Matt Wozniski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've simplified the change (to stop calling the callback as soon as we find the first occurrence of the symbol, instead of continuing to iterate through the modules and ignoring future occurrences of that symbol).
I've also simplified the test (we can reproduce this with ctypes, without needing to use llvmlite).
I've also added a stage to our CI that runs the test suite with a uv
installed interpreter (and verified that the new test fails with this interpreter if we don't make the fix).
And I've made two unrelated fixes to problems I noticed while working on this:
- I've made the test suite abort instead of hanging if
gcore
isn't installed - I've fixed a bug where a debug message printed out the Python version incorrectly
This now LGTM, but please check my work before landing this because I made a lot of changes to what you had.
When libraries like llvmlite dlopen the Python binary, it creates duplicate
mappings of the binary in the process memory. This caused pystack to use the
wrong address for _PyRuntime, leading to "Invalid address in remote process" errors.
The fix ensures we use the first (lowest address) mapping found, which is
typically the original/correct one, rather than overwriting with later mappings.
Fixes #255
Issue number of the reported bug or feature request: #
Describe your changes
A clear and concise description of the changes you have made.
Testing performed
Describe the testing you have performed to ensure that the bug has been addressed, or that the new feature works as planned.
Additional context
Add any other context about your contribution here.