[Python-Dev] Issue 14417: consequences of new dict runtime error

[R. David Murray]

On Thu, 29 Mar 2012 13:09:17 -0700, Guido van Rossum <guido at python.org> wrote:
> On Thu, Mar 29, 2012 at 12:58 PM, R. David Murray <rdmurray at bitdance.com> wrote:
> > Some of us have expressed uneasiness about the consequences of dict
> > raising an error on lookup if the dict has been modified, the fix Victor
> > made to solve one of the crashers.
> >
> > I don't know if I speak for the others, but (assuming that I understand
> > the change correctly) my concern is that there is probably a significant
> > amount of threading code out there that assumes that dict *lookup* is
> > a thread-safe operation. Â Much of that code will, if moved to Python
> > 3.3, now be subject to random runtime errors for which it will not
> > be prepared. Â Further, code which appears safe can suddenly become
> > unsafe if a refactoring of the code causes an object to be stored in
> > the dictionary that has a Python equality method.
> 
> My original assessment was that this only affects dicts whose keys
> have a user-implemented __hash__ or __eq__ implementation, and that
> the number of apps that use this *and* assume the threadsafe property
> would be pretty small. This is just intuition, I don't have hard
> facts. But I do want to stress that not all dict lookups automatically
> become thread-unsafe, only those that need to run user code as part of
> the key lookup.

You are probably correct, but the thing is that one still has to do the
code audit to be sure...and then make sure that no one later introduces
such an object type as a dict key.

Are there any other places in Python where substituting a duck-typed
Python class or a Python subclass can cause a runtime error in previously
working code?

> > Would it be possible to modify the fix so that the lookup is retried a
> > non-trivial but finite number of times, so that normal code will work
> > and only pathological code will break?
> 
> FWIW a similar approach was rejected as a fix for the hash DoS attack.

Yes, but in this case the non-counting version breaks just as randomly,
but more often.  So arguing that counting here is analogous to counting
in the DoS attack issue is an argument for removing the fix entirely :)

The counting version could use a large enough count (since the count
used to be infinite!) that only code that would be having pathological
performance anyway would raise the runtime error, rather than any code
(that uses python __eq__ on keys) randomly raising a runtime error,
which is what we have now.

--David