Skip to content
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

"mem" and "object" Allocators are No Longer Protected by the GIL #105766

Closed
ericsnowcurrently opened this issue Jun 14, 2023 · 3 comments
Closed
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API topic-subinterpreters type-bug An unexpected behavior, bug, or error

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jun 14, 2023

Once we moved to per-interpreter GIL, the promises in the docs no longer hold:

...where the allocation must be performed with the GIL held.

It's still fine for pymalloc, but any custom, non-wrapping "mem"/"object" allocators would need to be updated to be thread-safe or per-interpreter.

I have a PR up that does an okay job of adapting such allocators: gh-105619. However, it penalizes use of such an allocator in subinterpreters that have their own GIL.

Honestly, I'm leaning toward documenting that such allocators must be thread-safe or per-interpreter. From what I understand, the documented guarantees (in the docs and in PEP 445) are more about representing what pymalloc needs than what custom allocators need.

Perhaps the biggest question is: what projects would be impacted? I haven't had a chance yet to search for projects that use custom mem/object allocators that aren't thread-safe. I suspect there aren't more than two or three.

Linked PRs

@ericsnowcurrently ericsnowcurrently added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters topic-C-API 3.12 bugs and security fixes 3.13 bugs and security fixes labels Jun 14, 2023
@encukou
Copy link
Member

encukou commented Jun 14, 2023

Can we add a flag, so that e.g. PyMem_SetAllocator(PYMEM_DOMAIN_MEM | PyMEM_THREADSAFE, ...) would set PYMEM_DOMAIN_MEM without the wrappers?

@ericsnowcurrently
Copy link
Member Author

@ericsnowcurrently
Copy link
Member Author

I'm going to add a note in the docs that custom allocators (that have their own state), including hooks/wrappers, must be thread-safe.

We could also qualify that to only when subinterpreters are used, but I'm not sure that matters. If we did qualify it that way, it may also make sense to add the flag @encukou suggested above (and its inverse).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API topic-subinterpreters type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

2 participants