MAINT: fix thread-unsafe cache initialization in PyUFunc_TrueDivisionTypeResolver #26497
Conversation
|
LGTM, but will wait on @seberg. |
|
This LGTM, but I wanted to discuss it briefly tonight. We have a lot of similar initializations, so I think it would be much nicer to see if we cannot find a local pattern instead. |
|
FWIW, at least for |
|
Sorry, but I am not following. What is actually different about this one? Arguably, all it seems to do is one INCREF of an immortal object (fine) and then call |
|
The difference here is this one causes a memory leak, whereas |
|
Aha, right so one leaks only reference to things that are practically immortal while this one is a bit worse as it leaks a fresh tuple. We had discussed yesterday a bit and landed a bit on trying to consolidate this "static data" into a global struct (which makes it a step towards a module level struct for multi-phase init/sub-interpreters). Sounds like that might be worthwhile, but will also be a somewhat larger refactor. FWIW, I am OK with this if it seems easier or necessary right now, I still think it would be nice to use this delayed-init pattern slightly more rather than less so having a solution for it would be nice. (And I think the global struct will do that). |
|
Closing in favor of #26607 |
This moves initialization for a static tuple used in the division type resolver from inside the resolver body to the initialization for the umath module.
This is only really needed for the free-threaded build of Python. Initializing globals like this during module initialization is thread safe.
No test because I can't think of a way to cause breakage from the data race. Still fixing it since I'm globally auditing usages of static globals in the codebase.