-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
gh-116738: Test uuid module for free threading #140068
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
The memcpy() call in _PyInterpreterState_New() was overwriting the _malloced pointer that was set by alloc_interpreter(), causing a memory leak when subinterpreters were destroyed. Fixed by preserving the _malloced pointer across the memcpy().
cc @picnixz |
First of all, thanks a bunch for the detailed and thorough research!
I would lean towards not modifying the existing behavior in the with-GIL builds (either by adding a mutex in the free-threaded build, or by leaving the code unmodified, thereby allowing the builds to diverge). Curious to see what others think. |
Yes, I agree |
Agreed, also if the functions do not block then releasing and acquiring the GIL would cause a slowdown in the gil-enabled build. |
@mpage @colesbury @kumaraditya303 thanks for the comments! I’ve removed the |
This PR is more about explaining the reasoning behind the changes than about the code itself, which is actually quite minimal.
Starting with Windows (the easy one):
_uuid
module usesUuidCreateSequential()
. In the current GIL-enabled build, the GIL is already released while this function is called.UuidCreateSequential()
is thread-safe, it does mention that if the computer doesn’t have an Ethernet or token ring address, the generated UUID is still a valid identifier and is guaranteed to be unique among all UUIDs generated on that machine. So, at the very least, we have local uniqueness.UuidCreateSequential()
also be safe in a free-threaded build. I’ve added a test for it.For the rest of the systems (Linux, macOS, Solaris, AIX, FreeBSD, etc.):
uuid_generate_time_safe()
,uuid_create()
, oruuid_generate_time()
.libuuid (
uuid_generate_time_safe()
anduuid_generate_time()
):Based on all of the above, I changed the
uuid
module to release the GIL for these libuuid functions, since everything I found indicates they are thread-safe. I did come across some old reported issues, but those have either been fixed in the library itself or addressed in the distributions. However, if there are still concerns about thread safety, to be on the safe side we can removePy_{BEGIN,END}_ALLOW_THREADS
and add a mutex to protect these functions in the free-threading build.FreeBSD, OpenBSD and AIX (
uuid_create()
)I changed the
uuid
module to release the GIL forcreate_uuid()
. However, the situation on AIX is still unclear, and I’m not sure if there are any other systems that usecreate_uuid()
. It might be safer to removePy_{BEGIN,END}_ALLOW_THREADS
and protectcreate_uuid()
with a mutex in the free-threading build.Finally, just a note regarding
libuuid
: Even though the functions seem thread-safe, that doesn’t always mean the UUIDs will be unique. To guarantee uniqueness of UUIDs, libuuid first tries to use a global clock state file (if the process has permission) and/or the uuidd daemon (if it’s running or can be started). If those aren’t available, it falls back to getrandom(), then/dev/{u,}random
, and finallyrand()
. In the worst case, it’s theoretically possible for two processes running at the same time to generate the same UUID.cc: @mpage @colesbury @Yhg1s @kumaraditya303 @vstinner