Uh oh!
There was an error while loading. Please reload this page.
gh-150490: Raise PyType_Modified for insertion into split dictionary - #150489
Conversation
226cf29 to
8406f6fCompare
markshannon
left a comment
There was a problem hiding this comment.
The change to the dict and removal of the keys version check seems good, but why have you removed several seemingly unrelated tests?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| static PyDictKeysObject* | ||
| new_keys_object(uint8_t log2_size, bool unicode) |
There was a problem hiding this comment.
Why not pass the kind directly, instead of a bool?
There was a problem hiding this comment.
It's not actually changed the diff is just doing a bad job of seeing that part of this was pulled into init_keys_object so it's showing this both deleted and added. But the callers tend to have. the bool value already in hand and new_keys_object seems to want to do a couple of truthy checks so I think it makes sense.
markshannon
left a comment
There was a problem hiding this comment.
I was hoping that #142889 would be implemented first, but it looks like progress there has stalled.
So, let's merge this.
vstinner
commented
Jun 17, 2026
This change introduced a regression: |
…onary (python#150489) Raise PyType_Modified for insertion into split dictionary
Revert commits: gh-151593: Fix dead lock in PyDict insert_split_key() (#152200) gh-150490: Raise PyType_Modified for insertion into split dictionary (#150489) For gh-150489, it violates locking discipline and results in deadlocks, gh-151593 is an example of it being hit in CI. The attempted fixgh-152200 avoids the deadlock but introduces a data-race. The race window is small but can be triggered with pure Python code.
When we insert into a split dictionary we update the shared keys version - this is used to invalidate caches for loading methods and loading class values and requires us to check the keys version. Instead we can raise PyType_Modified which lets us rely on the type version check + has inline values check instead of validating that we have the correct keys version. This gets rid of loading the cached keys version, the objects type (although likely the compiler eliminates this already), loading the cached keys from the type, and then loading the keys version and comparing it against the cached value.