Fix threading-related deadlocks, and also add support for Python's built-in listing of libraries - #243
Conversation
itamarst
changed the title
|
May have figured how to test deadlocks; it only happens on Conda! |
|
So the test is demonstrating I didn't solve deadlocks on Python 3.13 and earlier (3.14 I'm using the new built-in API which is written in C, so GIL doesn't get released at in-opportune times). I'm starting to think having a Python callback (as opposed to C callback) to |
…o a mechanism that avoids the locks.
|
uOK, came up with another solution: completely bypassing |
|
@ogrisel OK this should be ready for review and hopefully allow for a release. |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
Hope you don't mind the drive-by review. Itamar linked this PR to me privately.
| Uses a Linux-specific mechanism: | ||
| https://man7.org/linux/man-pages/man5/proc_pid_maps.5.html | ||
| """ | ||
| with open("/proc/self/maps") as f: |
There was a problem hiding this comment.
I think you probably want to open in "rb" mode, otherwise a non-UTF-8 filename would break here.
There was a problem hiding this comment.
This matches previous behavior I think. And to be fair one could do better, but... not sure how one would know the actual encoding. And also in practice probably no one ever does that?
There was a problem hiding this comment.
UNIX paths don’t have any encoding, so just leaving everything as bytes should be fine. But also if it’s a preexisting issue it’s no biggie.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
Note that this PR does not fix #225 directly but it will likely reduce the size of the diff if we want to fix it. |
joblib#243 already lists libraries from /proc/self/maps. Keep that path and only avoid importing ctypes.util (load libc via CDLL(None), lazy-import dllist off Linux) so the libffi abort after fork from joblib#225 stays fixed. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve conflicts with joblib#243 by keeping Windows on the Toolhelp snapshot enumerator. ctypes.util.dllist still uses EnumProcessModules, which is the race this PR fixes. Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
Thank you! |
3 participants
Fixes #239
Also adds usage of Python's built-in logic for listing shared libraries.