BUG: apply critical sections around populating the dispatch cache#27400
BUG: apply critical sections around populating the dispatch cache#27400ngoldbaum merged 2 commits intonumpy:maintenance/2.1.xfrom
Conversation
|
@ngoldbaum Might check |
This comment has been minimized.
This comment has been minimized.
|
@ngoldbaum Looks like |
|
Oh sorry about my comment earlier, I thought you were commenting on the original PR. This is definitely wrong, the critical section API is defined with macros that include braces so you can't put them in error handling sections. I'm not sure why the backport didn't apply cleanly - if you don't mine waiting I can probably push the correct backport to this PR sometime tomorrow. I'm out sick today. |
Sebastian cleaned out the legacy promotion mode, so the protected section is much smaller in main. |
96fe642 to
393b08c
Compare
|
I can see why this solution wasn't obvious to me back in June! Thanks for working through this. This looks correct to me. I tried reproducing the error in the |
Backport of #27392.
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_dtypesCI 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-compatlets us use the critical section macros without putting them insidePy_GIL_DISABLEDmacros. 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.