Roundup Robot devn...@psf.upfronthosting.co.za added the comment:
New changeset 93748e2d64e3 by Antoine Pitrou in branch 'default':
Issue #14417: Mutating a dict during lookup now restarts the lookup instead of
raising a RuntimeError (undoes issue #14205).
Antoine Pitrou pit...@free.fr added the comment:
I have committed an updated patch, with a fix to Victor's test. I don't think
anything else remains to be done.
--
resolution: - fixed
stage: - committed/rejected
status: open - closed
___
Python
Guido van Rossum gu...@python.org added the comment:
Thanks Antoine!
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
___
Guido van Rossum gu...@python.org added the comment:
I could check it in, but I probably would mess up something (which branches are
affected?). Let me know if you want me to.
The priorities after that would be:
1) update docs (the warning about RuntimeError needs to be moderated)
2) convert
Guido van Rossum gu...@python.org added the comment:
Actually my patch doesn't even apply cleanly. I suspect the dict refactoring
for shared keys interfered. Someone please help!
--
___
Python tracker rep...@bugs.python.org
Guido van Rossum gu...@python.org added the comment:
Still no progress on this bug. Should I just check in my simple patch? But
there's much more to do -- docs, and unittests. Volunteers? It's not hard, just
work.
--
___
Python tracker
Antoine Pitrou pit...@free.fr added the comment:
Still no progress on this bug. Should I just check in my simple patch?
But there's much more to do -- docs, and unittests. Volunteers? It's
not hard, just work.
Well, in general the person writing the patch should also write the
tests ;-)
I
STINNER Victor victor.stin...@gmail.com added the comment:
A counter can be added to dictobject.c.patch to avoid an infinite loop.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
Guido van Rossum gu...@python.org added the comment:
A counter can be added to dictobject.c.patch to avoid an infinite loop.
But why bother? There was no counter originally -- it would continue
until it ran out of stack. Since this can only be triggered if there
is Python code in the __eq__,
Antoine Pitrou pit...@free.fr added the comment:
Does this mean I should just check it in? But I asked, and never got
an answer, whether the original stress test had been converted into a
unittest. I'd like that to happen before I check this in. Also there
are probably docs I've missed.
Jim Jewett jimjjew...@gmail.com added the comment:
A counter can be added to dictobject.c.patch to avoid
an infinite loop.
But why bother?
Without a termination condition, how is this different from just reverting the
original change? Is it just the slightly improvement in efficiency?
Antoine Pitrou pit...@free.fr added the comment:
Without a termination condition, how is this different from just
reverting the original change?
The difference is that it's a (presumably interruptible) infinite loop,
not a stack overflow. There's no crash and therefore no security issue.
STINNER Victor victor.stin...@gmail.com added the comment:
The difference is that it's a (presumably interruptible) infinite loop,
not a stack overflow. There's no crash and therefore no security issue.
Ok, it looks fair. The patch looks good, but as written by Antoine: tests may
need to be
Antoine Pitrou pit...@free.fr added the comment:
But the sleep(0.1) forces a thread switch so I consider that still
cheating -- nobody in their right mind would consider calling sleep()
inside __hash__.
Well, cheating is fair game when trying to test borderline cases, isn't
it?
--
Stefan Krah stefan-use...@bytereef.org added the comment:
OK, here's a version with a low switch interval. Of course it's also
contrived, but it works.
Generally I'd appreciate the RuntimeError, since it's a hint that
something needs to be rewritten in an application.
It might be a problem
Antoine Pitrou pit...@free.fr added the comment:
OK, here's a version with a low switch interval. Of course it's also
contrived, but it works.
The drawback of using setswitchinterval() is that it makes the test less
reusable by other implementations (or perhaps it will succeed without
R. David Murray rdmur...@bitdance.com added the comment:
Antoine: I don't think the point of this code is to come up with a unit (or
other) test for the behavior, but to try to determine empirically whether or
not this error is likely to be an issue in naive production code (whether it is
Antoine Pitrou pit...@free.fr added the comment:
Antoine: I don't think the point of this code is to come up with a
unit (or other) test for the behavior, but to try to determine
empirically whether or not this error is likely to be an issue in
naive production code (whether it is existing
Raymond Hettinger raymond.hettin...@gmail.com added the comment:
Well, cheating is fair game when trying to test
borderline cases, isn't it?
It is fair game (and necessary) when it comes to
exposing bugs that are hard to reproduce.
I'm wary of the original RuntimeError patch because
* it
Nick Coghlan ncogh...@gmail.com added the comment:
A thought prompted by Raymond's comment: did we ever try just protecting
the retry as a recursive call? If we can stop the stack blowing up, it
seems to me we'd get the best of both worlds (that is, crashes become
RuntimeError, but naive
Guido van Rossum gu...@python.org added the comment:
On Sun, Apr 1, 2012 at 1:58 PM, Raymond Hettinger
rep...@bugs.python.org wrote:
[...]
I'm wary of the original RuntimeError patch because
[...]
I had retorts to most of what you wrote, but decided not to post them.
Instead, I really want to
Guido van Rossum gu...@python.org added the comment:
Here's a hack that uses goto instead of recursion to restore the original
behavior, less the stack overflow. With this, hammer_dict_switchinterval.py
loops forever (which is I think what it's supposed to do if RuntimeError is
never
Raymond Hettinger raymond.hettin...@gmail.com added the comment:
IIRC, Jython uses concurrent mappings, so this isn't an issue for them.
CPython's dictresize() relies on the GIL to atomically resize the ma_table.
There are no pure python calls (the existing hash values are reused) and the
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
--
Removed message: http://bugs.python.org/msg157338
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
Guido van Rossum gu...@python.org added the comment:
Why delete that?
On Sunday, April 1, 2012, Raymond Hettinger wrote:
Changes by Raymond Hettinger raymond.hettin...@gmail.com javascript:;:
--
Removed message: http://bugs.python.org/msg157338
Nick Coghlan ncogh...@gmail.com added the comment:
Attached script is a first cut at a multi-threading stress test for the new
dict behaviour. It should be significantly worse than anything a real world app
is likely to be doing to a dictionary that is shared between threads without
any form
R. David Murray rdmur...@bitdance.com added the comment:
Thanks, Nick, this is much better than speculation.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:
--
nosy: +Arfrever
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
Stefan Krah stefan-use...@bytereef.org added the comment:
By adding a slow __eq__() method to Nick's script I can trigger the
RuntimeError reliably.
--
nosy: +skrah
Added file: http://bugs.python.org/file25086/hammer_dict_eq.py
___
Python tracker
Guido van Rossum gu...@python.org added the comment:
But the sleep(0.1) forces a thread switch so I consider that still cheating --
nobody in their right mind would consider calling sleep() inside __hash__.
You can probably get it to fail without this by just doing a bunch of random
Changes by Gregory P. Smith g...@krypto.org:
--
nosy: +gregory.p.smith
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
___
Antoine Pitrou pit...@free.fr added the comment:
I must admit to being concerned by the possible impact of this change as well.
So am I.
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
Guido van Rossum gu...@python.org added the comment:
On Thu, Mar 29, 2012 at 9:12 AM, Antoine Pitrou rep...@bugs.python.org wrote:
Antoine Pitrou pit...@free.fr added the comment:
I must admit to being concerned by the possible impact of this change as
well.
So am I.
I think it's time
Changes by Andrew Svetlov andrew.svet...@gmail.com:
--
nosy: +asvetlov
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
___
New submission from Jim Jewett jimjjew...@gmail.com:
Per the 3.3 WhatsNew:
issue 14205: A dict lookup now raises a RuntimeError if the dict is modified
during the lookup. If you implement your own comparison function for objects
used as dict keys and the dict is shared by multiple threads,
R. David Murray rdmur...@bitdance.com added the comment:
I must admit to being concerned by the possible impact of this change as well.
--
nosy: +r.david.murray
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: +gvanrossum
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
___
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: +haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14417
___
___
Python-bugs-list
38 matches
Mail list logo
