Skip to content

Inheriting from collections.MutableMapping and typing.MutableMapping breaks MutableMapping.register #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
euresti opened this issue May 17, 2016 · 3 comments

Comments

@euresti
Copy link

euresti commented May 17, 2016

import collections
from typing import MutableMapping, TypeVar, Generic

_KT = TypeVar('_KT')
_VT = TypeVar('_VT')

class MyMutableMapping(collections.MutableMapping, MutableMapping[_KT, _VT], Generic[_KT, _VT]):
    pass

class OtherMutableMapping(object):
    pass

collections.MutableMapping.register(OtherMutableMapping)

Running this code under python2 results in the following error:

raceback (most recent call last):
  File "t2.py", line 13, in <module>
    collections.MutableMapping.register(OtherMutableMapping)
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 109, in register
    if issubclass(subclass, cls):
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 180, in __subclasscheck__
    if issubclass(subclass, scls):
  File "/usr/local/lib/python2.7/site-packages/typing.py", line 1056, in __subclasscheck__
    return issubclass(cls, self.__extra__)
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 180, in __subclasscheck__
    if issubclass(subclass, scls):
  File "/usr/local/lib/python2.7/site-packages/typing.py", line 1056, in __subclasscheck__
    return issubclass(cls, self.__extra__)
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 180, in __subclasscheck__
<snip>
    if issubclass(subclass, scls):
  File "/usr/local/lib/python2.7/site-packages/typing.py", line 1056, in __subclasscheck__
    return issubclass(cls, self.__extra__)
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 180, in __subclasscheck__
    if issubclass(subclass, scls):
  File "/usr/local/lib/python2.7/site-packages/typing.py", line 1056, in __subclasscheck__
    return issubclass(cls, self.__extra__)
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py", line 151, in __subclasscheck__
    if subclass in cls._abc_cache:
RuntimeError: maximum recursion depth exceeded
@bintoro
Copy link
Contributor

bintoro commented May 18, 2016

The issue is not specific to register. Inheriting from the collections and typing variants of the same collection at the same time is not really safe because it breaks subsequent subclass checks against either ABC. (Checks that immediately return True or are in the cache will still work, but others won't.)

The registration attempt results in issubclass(OtherMutableMapping, collections.MutableMapping) as a sanity check. Since there is no direct match, the query is delegated to any subclasses of collections.MutableMapping, which now include MyMutableMapping.

The resulting issubclass(OtherMutableMapping, MyMutableMapping), in turn, leads back to the original call via the __extra__ linkage from MyMutableMapping to collections.MutableMapping; hence the infinite recursion.

The problem is fixed in the first commit of #207.

@gvanrossum
Copy link
Member

@bintoro: Thanks for reminding me of that! I've been wanting to check whether your commit would fix this (or, actually, http://bugs.python.org/issue27014, but I think that's the same issue).

I still don't have the time or the stomach for the rest of your PR #207 -- I wonder if you could resubmit just the first commit, assuming it doesn't do much more than fix this particular issue? (If you don't have the time I can probably figure out how to cherry-pick it; the main issue I expect is the merge conflict with e599f84.

@bintoro
Copy link
Contributor

bintoro commented May 18, 2016

Yes, 27014 is the same issue. And yeah, I can split off the first commit, since it only adds a few tests. I'm not terribly motivated to deal with the rest of the conflict, either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants