Popkey ordereddict7/21/2023 ![]() Make popitem more subclass friendly when it's an OrderedDict subclass, instead of using _odict_LAST and _odict_FIRST, use (C equivalent) of next(reversed(self)) and next(iter(self)) respectively.The del self works because they didn't override _iter_, so it's not expiring anything to get the first item, and therefore only _delitem_ is involved, not _contains_ or _getitem_ (note: This is also why the bug they reference has an issue with "OrderedDict mutated during iteration" iteration returns long expired keys, but looking the expired keys up deletes them, causing the mutation issue). The failing popitem call is in _setitem_ for limited length expiringdicts, self.popitem(last=False), where they're intentionally removing the oldest entry, when the oldest entry is the most likely to have expired (and since _len_ isn't overridden to expire old entries, it may have been expired for quite a while). At time X+1000 or so, the PySequence_Contains check is run, and it returns 0 (false), because the _contains_ machinery is invoked, and again, because no default is provided for popitem, a KeyError is raised (this time by the popitem machinery, not _getitem_)Įxpiringdict is unusually good at bringing this on itself.At time X+1000, _odict_FIRST (in popitem) grabs the first entry in the OrderedDict without invoking the _contains_ machinery that would delete the entry. ![]() No lookups or membership tests or any other operations that implicitly clean the expiring dict occur for a while.Because contains returned True, at time X PyObject_GetItem is run, but because it's time X, expiringdict's _getitem_ deletes the entry and raises KeyErrorĪn alternative scenario with a *huge* race window is:.At time X-1, the PySequence_Contains check is run, and it returns 1 (true).An entry has an expiry time of X (so it will self-delete at time X or later).dict happily bypasses custom _getitem_/ delitem calls when it uses pop/popitem.Įxplaining expiringdict's issue: It's two race conditions, with itself, not another thread. The only reason OrderedDict has the problem and dict doesn't is that OrderedDict was trying to be subclassing friendly (perhaps to ensure it remains compatible with code that subclassed the old Python implementation), while dict makes no such efforts. The Sequence_Contains check is needed to avoid accidentally invoking _missing_ (though if _missing_ is not defined for the subclass, the Sequence_Contains check could be skipped). The general PyObject_GetItem/DelItem are needed to work with arbitrary subclasses correctly. ![]() The original code should probably be restored here. ![]() The expiringdict's flaw seems to be that its _contains_ call and its _getitem_ are not idempotent, which the original code assumed (reasonably) they would be. _setitem_ might add some tracking data to the value that _getitem_ strips). The old code meant that pop and popitem didn't need to be overridden even if you overrode _getitem_/ delitem in a way that differed from the default (e.g. Serhiy, doesn't this patch "fix" the issue by making subclasses with custom _getitem_/ delitem implementations not have them invoked by the superclass's pop/popitem? Priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = '' versions = Activity = actor = 'lukasz.langa' assignee = 'serhiy.storchaka' closed = True closed_date = closer = 'lukasz.langa' components = Ĭreation = creator = 'kaniini' dependencies =
0 Comments
Leave a Reply. |