[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: > Yeah, I stopped it at 300k iterations. No problems. Glad this fix got in for rc1. Thanks for the double-check, Larry. I'd done similar testing, though not as exhaustively. :) Getting a fix in for RC1 was pretty important to me. It stretched me a bit, but I got

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Larry Hastings added the comment: Yeah, I stopped it at 300k iterations. No problems. Glad this fix got in for rc1. -- ___ Python tracker ___ _

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Larry Hastings added the comment: FWIW I set up a test harness that runs test_collections and odict_reproduce with monotonically increasing PYTHONHASHSEED values. I let it run overnight; it's now past iteration 158600. Apart from some weirdness in the low 70ks that I can't reproduce it's bee

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: I've pushed the fix for RC1. Thanks again Fabian for bringing it to our attention. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker _

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Roundup Robot added the comment: New changeset adb510322c8f by Eric Snow in branch '3.5': Issue #24667: Resize odict in all cases that the underlying dict resizes. https://hg.python.org/cpython/rev/adb510322c8f New changeset 47287c998bb0 by Eric Snow in branch 'default': Merge from 3.5 (issue #2

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: That's good to know. Thanks. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Okay I think I've run the test suite on 3.6 although it only works with https://gerrit.wikimedia.org/r/#/c/229683/ now as inspect.getargspec has been removed (when I initially posted this, it was just deprecated). Well writing this I just thought I could've tested tha

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: If I don't get any feedback before then, I'll commit the patch on Friday. -- ___ Python tracker ___ ___ P

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: @Fabian, hey, thanks for bringing it to our attention! -- ___ Python tracker ___ ___ Python-bugs-list mai

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Changes by Yury Selivanov : -- nosy: +yselivanov ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.p

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Thank you for figuring it out and providing a patch after I couldn't really give good information. I haven't been able to test it for myself but your explanation makes sense :) -- ___ Python tracker

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Changes by Eric Snow : Removed file: http://bugs.python.org/file40126/odict-correct-resize.diff ___ Python tracker ___ ___ Python-bugs-list ma

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Changes by Eric Snow : Added file: http://bugs.python.org/file40127/odict-correct-resize.diff ___ Python tracker ___ ___ Python-bugs-list mail

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Changes by Eric Snow : -- priority: normal -> release blocker ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: It turns out the problem was that the odict resize mechanism was not getting triggered in all the cases that it should have been. dict resizes after a certain number of insertions, whether or not previous deletions have cleared out space. odict only resizes its f

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: It's looking like the problem is in _odict_add_new_node. If that's the case then I should be able to resolve this issue soon. -- ___ Python tracker ___

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: I've verified that it is definitely the linked list that is getting updated incorrectly at the point that a key is popped off. The underlying dict is working fine. The erroneous behavior is happening with either pop, popitem, or __delitem__. However, it is likel

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: For the reproducer I'm sticking with a seed of 1: PYTHONHASHSEED=1 ./python /tmp/odict_reproduce.py -- ___ Python tracker ___

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Changes by Eric Snow : Added file: http://bugs.python.org/file40078/odict_reproduce.py ___ Python tracker ___ ___ Python-bugs-list mailing lis

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: As expected (considering the link to dict ordering here), the inconsistent results are tied to hash randomization: $ for i in `seq 1 25`; do echo $i; PYTHONHASHSEED=$i ./python /tmp/odict_reproduce.py; done

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Getting closer. Here's a reproducer derived from the calls made while running the pywikibot test suite (3 tests only). Note that the KeyError indicates a different key for each run, even though the code is consistent. This means that order preservation is failin

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Also, here is the output from running the pywikibot suite (using just the 3 test modules) with the logging odict wrapper. -- Added file: http://bugs.python.org/file40069/odict.log ___ Python tracker

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: After some quick tests, it looks like during the tests only a single thread is used, so it is *not* threading-related. That makes a bit more sense to me given the consistency. Notwithstanding this development, I should be able to isolate the problem soon (when I

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: It's starting to look like this is threading-related. There's a single requests.Session defined as a module global in pywikibot.comms.http which is shared by all threads handling requests. requests.Session uses OrderedDict for an LRU cache for redirects. That's

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: I've been able to pare down the tests that run to reproduce the bug. These 3 must run: archivebot interwiki_graph l10n Given how long the entire suite takes, this helps drastically improve my iteration time while debugging. At this point I should be able to more

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Ah okay. If you look at Eric's response you'll see that this is a different issue and tracked in issue24721. I mean it is possible that urllib3 does it too (it's devs would need to answer that) but in the end highly unlikely as it doesn't happen on versions before 3.5

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Larry Hastings added the comment: Mark's test case file produced a KeyError under 3.4 when I tried it. -- ___ Python tracker ___ ___ P

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Really this bug and not the one mentioned by Mark? I've been using 3.4(.3) since we use requests (and therefore urllib3) which happened a few months ago. I also can't remember any failure on a Python 3.4 Travis build. -- __

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Larry Hastings added the comment: It produces the same exception under Python 3.4, too. -- nosy: +larry ___ Python tracker ___ ___ Pyt

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Regarding this bug, it's clear now that the ordered keys and the underlying dict are getting out of sync somewhere. This is either due to a bug in the C OrderedDict implementation or the use of the concrete dict C-API (or dict.* methods; thanks Mark). It's undoub

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: @Mark, note that you get the same behavior with the pure Python OrderedDict. Calling dict.* methods on an OrderedDict gives you undefined behavior. I expect the same is true for most subclasses of builtin types that override builtin methods. Anyway, the problem

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Mark Shannon added the comment: The attached test case raises a KeyError for __str__() -- nosy: +Mark.Shannon Added file: http://bugs.python.org/file40019/test.py ___ Python tracker

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: That worked. I'll take a close look at what's going on as soon as I can. -- ___ Python tracker ___ ___ P

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Just as a note to the tests: You may not get the issues with OrderedDict as a failure/error at the end of the test suite. And you may (depending on the version) get a few errors because NoUsername was raised. That is unrelated to this issue and can be fixed by using a

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Oh sorry, I basically never need to install pywikibot anew so it's easy to forget but there is a submodule in scripts/i18n which needs to be cloned as well. With the following commands I could reproduce the error (and you don't even need to install requests and six):

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: I've thus far been unsuccessful in running the pywikibot test suite. I'm guessing there are some prerequisites (e.g. an account on some wiki site). Is there a way to run the tests without network access? Also, I ran into some trouble with i18n.__file__, but hack

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Okay I did a test run on all three 3.5 betas available to me through pyenv. The beta 3 failed as Python 3.6 does that popitem() raises a KeyError. The beta 2 had the bug that popitem() does not support keyword arguments so I wasn't able to test it there. And the beta

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Yes see the tests/README.rst. And afaik do you only need to have requests and six installed. -- ___ Python tracker ___

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Thanks for the extra info. I'm going to see if I can reproduce the issue by running the pywikibot test suite locally. What's the best way to set that up? Are there instructions somewhere? As to the C implementation, it was first released (as a special exception)

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: It is consistent as in it happens on every run of the test suite. But unfortunately I haven't checked if it's always happening at the same place. Luckily we have 4 builds on Travis with 3.6 and in all it happened from the beginning and got > 100 matches for “KeyError:

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Ah, sorry. I wasn't thinking past Python 3.5 (which is about to go to beta 4). While 3.6.0a0, doesn't tell us much, d6c91b8242d2 (r96935) does. That revision has all the necessary fixes to OrderedDict. I'll look into this some more. Do you get the failure cons

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Well as I described in the opening post: "I don't think versions before Python 3.6 are affected as we had tests running on Python 3.5 (before Travis switched to 3.6 recently) and these all worked." Now tbh I don't know if a version of 3.5 is affected but was never tes

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Correct me if I'm wrong but the travis-ci logs [1] seem to indicate it's using Python 3.6.0a0. [1] https://travis-ci.org/xZise/pywikibot-core/builds/71550286#L144 -- ___ Python tracker

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Eric Snow added the comment: Just to get this out of the way, are you running your tests against the latest beta (3)? There were some known bugs in earlier betas that have since been fixed. -- ___ Python tracker

[issue24667] OrderedDict.popitem()/__str__() raises KeyError

Fabian added the comment: Looking further into this issue, OrderedDict.pop() using the key returned from the KeyError (using eval(str(error))) also yields a KeyError. And OrderedDict.popitem() does not change the dictionary (so it's not like the KeyError is raised even though it worked). Also