FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Race between UserDict.__getitem__ and __delitem__ can result in __missing_ not being called when item is missing. · Issue #156544 · python/cpython · GitHub

Repository navigation

Race between UserDict.__getitem__ and __delitem__ can result in __missing_ not being called when item is missing. #156544

Open

Description

if key in self.data:

I believe there is a race if __getitem__ sees the key exists in self.data and the GIL is released before getting the item. If another thread then calls __delitem__ which removes the item from self.data, when __getitem__ resumes it will attempt to get and return the item which no longer exists resulting in KeyError being raised out of __getitem__. If the class has __missing__ defined it should be called if the item is missing, but that won't occur in this sequence of events.
This is a theoretical bug report based solely on code inspection prompted by a DPO discussion about a different race the GIL does protect against (https://discuss.python.org/t/pep-805-safe-parallel-python/108670/49).

The only concern is that __missing__ will not be called, not that the item is initially seen then not seen.
I have not done a thorough review yet of the rest of the UserDict methods that do a containment check followed by lookup, but my cursory inspection they look to delegate to __getitem__. I am a bit concerned that get() does not call __missing__ if the initial check does not see the item but will if it sees the item and this bug is fixed as proposed below will result in __missing__ being called. This should be consistent.
#17910 seems related and I will look into the details of that fix to ensure this fix is consistent (if I end up working on this issue).

I offer a strawman proposal for fixing this by changing __getitem__ to use a single inspection of self.data with something like:
item = self.data.get(key, MISSING) if item is not MISSING: return item ...
Where MISSING is an internal object guaranteed to not be a key in self.data (adding it if something suitable doesn't already exist).

I volunteer to work on this issue. To point me in the right direction are there existing tests (either in collections or elsewhere) that force GIL timing races I can look at as an example of how to write a test to verify this issue is fixed? The challenge is reliably interleaving the initial containment check, then __delitem__, then the subscript access. I'm not sure if this level of testing is feasible based on previous experience with similar issues...the fix usually invalidates the test steps to interleave things properly by removing the ability for them to be interleaved. Any guidance on this would be very appreciated if my offer to work on this is taken up.

Linked PRs

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    stdlibStandard Library Python modules in the Lib/ directory
    on Aug 29, 2026
  2. markshannon commented on Aug 29, 2026

    Member

    Note: this is a bug in the free-threading build. The code is correct with the GIL.

  3. gdchinacat commented on Aug 29, 2026

    Author

    I can trivially reproduce on 3.14t. The only way I have found to reproduce a race on 3.14+gil was to enable tracing with sys.settrace() and threading.settrace(), and I don't consider that something worth worrying about. I agree with @markshannon that this race is isolated to free-threading.

    My remaining concern is the different behavior between GIL and FT. Unlocked concurrent dict through subscripts is guaranteed to provide internal consistency and unlocked concurrent access is common. Since users don't always know the dict they are working with is a UserDict that implemented __missing__ that currently requires external locking to ensure documented functionality works correctly, I think this would be good to fix. I will spend some more time looking investigating the feasibility of this.

    Any additional insights into this would be very appreciated. I am concerned that my strawman proposal of how to fix may undo the previous fix to make UserDict.get() consistent with dict.get() with respect to __missing__ or may change how subclasses of UserDict that implement __getitem__ or __contains__ may see different behavior (as they did with the previous fix - #105524) and will check these issues specifically. Anything else I should look into?

  4. deleted a comment from on Aug 31, 2026
  5. gdchinacat commented on Sep 1, 2026

    Author

    @rhettinger After looking into the code and past fixes and discussions, it is not clear that this is even considered a bug. Basically, there are so many ways the UserDict (or, more accurately MutableMapping) methods that correspond to dict methods that have historically been atomic under the GIL (and as best I can tell still mostly are under free-threading) can be racy I'm wondering if fixing this one is worthwhile. I don't believe there is any way to fix them all without adding expensive locks that would hurt single-threaded performance in ways that practicality over purity can't justify to fix unlocked free-threaded code.
    In order to maintain semantics with respect to UserDict.data interactions in __getitem__ the fix I'm currently considering is:

        def __getitem__(self, key):
            if key in self.data:
                try:
                    return self.data[key]
                except KeyError:
                    pass
            if hasattr(self.__class__, "__missing__"):
                return self.__class__.__missing__(self, key)
            raise KeyError(key)
    

    Based on logic and testing I believe this will ensure UserDict.__getitem__ will always call __missing__ if it is defined even when there are concurrent calls to __delitem__ for the same key. This preserves the current semantics between UserDict and UserDict.data and does not materially hurt performance except when the race happens.

    Is this worth pursuing further? I think the best arguments are the race (mostly) does not exist under GIL and is trivial to reproduce under free-threading and raising KeyError without calling __missing__ does not conform to the docs. But...is it reasonable to expect unlocked concurrent access to behave the same with and without the GIL, or to ensure __missing__ is called? I just don't have a good sense for what the "official" position on these is. Any insight or opinions you can provide would be helpful. If you say "not a bug" I will be on board, but if you think this is worth fixing I'm also happy to submit a PR. Further discussion about what I've found would also be welcome, but want to respect your time if this is a non-starter. Thank you!

  6. markshannon commented on Sep 1, 2026

    Member

    @gdchinacat Can you wrap __getitem__ in backticks so it looks like this: __getitem__, not this: getitem

  7. markshannon commented on Sep 2, 2026

    Member

    UserDict is not documented as being either thread-safe or not thread-safe. I think it is reasonable to assume that implies thread safety; dict is thread-safe and UserDict mimics dict.
    Plus, UserDict is thread-safe with the GIL, and has been for years.

  8. added 2 commits that reference this issue on Sep 10, 2026
  9. rhettinger commented on Sep 21, 2026

    Contributor

    I think it is reasonable to assume that implies thread safety

    In general, the opposite is true. Thread-safety for pure python code is the exception instead of the rule.

  10. rhettinger commented on Oct 8, 2026

    Contributor

    After looking into the code and past fixes and discussions, it is not clear that this is even considered a bug.

    Personally, I do not consider this a bug.

    In the general case, most pure python code cannot be easily defended against concurrent updates without using locks. If that is something a user wants, it is something they can add as needed.

  11. gdchinacat commented on Oct 9, 2026

    Author

    @rhettinger Thanks for looking at this issue and giving your perspective. I agree with your assessment that locks are generally required to ensure consistency. My motivation for the PR is that in this specific case UserDict semantics with regard to __missing__ can be aligned with dict in the library without locks, performance degradation, or a change in UserDict semantics. This fix (to the extent it is considered a bug to be fixed) could prevent issues in cases where locking is not used due to oversight or incorrect assumptions that the mapping is a UserDict rather than a dict. It is an entirely theoretical bug, I don't know of any cases where this has caused actual issues. However, I don't think that necessarily implies it shouldn't be addressed. You have far more experience with this code and how it is used than I do, so if your determination is the risk of the proposed change outweighs the potential benefits, or the increased complexity (IE "why do both LBYL and EAFP"?) I will understand. If that's the case the bug should be closed as not a bug or won't fix; let me know if you want me to close it. Again, thanks for your consideration of this issue and the PR.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytopic-free-threadingtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions


      Back | FazBrowse Home | New Git URL