Skip to content

BUG: apply critical sections around populating the dispatch cache#27392

Merged
charris merged 2 commits intonumpy:mainfrom
ngoldbaum:ufunc-cache-fix
Sep 14, 2024
Merged

BUG: apply critical sections around populating the dispatch cache#27392
charris merged 2 commits intonumpy:mainfrom
ngoldbaum:ufunc-cache-fix

Conversation

@ngoldbaum
Copy link
Member

@ngoldbaum ngoldbaum commented Sep 13, 2024

Fixes #27386.

This moves the locking to a higher conceptual level in the code. Now we lock the entire ufunc object whenever we go into promote_and_get_info_and_ufuncimpl.

In my testing I'm not able to reproduce the duplicate identity cache entry error @jakevdp saw in the ml_dtypes CI locally after making this change. Unfortunately it's a multithreaded issue and inherently flaky, so I can't be 100% positive this fixes it.

Also side benefit of being a lot simpler!

Updating pythoncapi-compat lets us use the critical section macros without putting them inside Py_GIL_DISABLED macros. They're just open and close brace on the GIL-enabled build.

One question for @seberg or maybe @mhvk: is dtype promotion ever re-entrant? I don't think so but I'd like to double-check because I think it might be problematic if we ever recursively created critical sections here.

@ngoldbaum ngoldbaum added the 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703) label Sep 13, 2024
@ngoldbaum ngoldbaum added the 09 - Backport-Candidate PRs tagged should be backported label Sep 13, 2024
@ngoldbaum ngoldbaum changed the title BUG: apply critical sections around populating the ufunc cache BUG: apply critical sections around populating the dispatch cache Sep 13, 2024
@charris charris merged commit b307a8c into numpy:main Sep 14, 2024
@charris
Copy link
Member

charris commented Sep 14, 2024

Thanks Nathan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: populating the identity cache is not thread safe

2 participants