Skip to content

Commit

Permalink
Fix CPython3.5 crashes.
Browse files Browse the repository at this point in the history
Apparently there's a bug in tuple(map(func, it)) when func can return transient objects.

The `_lookup` function was overwriting the `cache` stack variable with garbage when `tuplefy` was called with such a map object. The fix is to stop using map in favor of list comprehensions.

This has the bonus of being faster.
  • Loading branch information
jamadden committed Mar 8, 2020
1 parent fc967f8 commit 32e25b3
Showing 1 changed file with 23 additions and 7 deletions.
30 changes: 23 additions & 7 deletions src/zope/interface/adapter.py
Expand Up @@ -30,6 +30,22 @@
'VerifyingAdapterRegistry',
]

# ``tuple`` and ``list`` cooperate so that ``tuple([some list])``
# directly allocates and iterates at the C level without using a
# Python iterator. That's not the case for
# ``tuple(generator_expression)`` or ``tuple(map(func, it))``.
##
# 3.8
# ``tuple([t for t in range(10)])`` -> 610ns
# ``tuple(t for t in range(10))`` -> 696ns
# ``tuple(map(lambda t: t, range(10)))`` -> 881ns
##
# 2.7
# ``tuple([t fon t in range(10)])`` -> 625ns
# ``tuple(t for t in range(10))`` -> 665ns
# ``tuple(map(lambda t: t, range(10)))`` -> 958ns
#
# All three have substantial variance.

class BaseAdapterRegistry(object):

Expand Down Expand Up @@ -114,7 +130,7 @@ def register(self, required, provided, name, value):
self.unregister(required, provided, name, value)
return

required = tuple(map(_convert_None_to_Interface, required))
required = tuple([_convert_None_to_Interface(r) for r in required])
name = _normalize_name(name)
order = len(required)
byorder = self._adapters
Expand Down Expand Up @@ -143,7 +159,7 @@ def register(self, required, provided, name, value):
self.changed(self)

def registered(self, required, provided, name=u''):
required = tuple(map(_convert_None_to_Interface, required))
required = tuple([_convert_None_to_Interface(r) for r in required])
name = _normalize_name(name)
order = len(required)
byorder = self._adapters
Expand All @@ -162,7 +178,7 @@ def registered(self, required, provided, name=u''):
return components.get(name)

def unregister(self, required, provided, name, value=None):
required = tuple(map(_convert_None_to_Interface, required))
required = tuple([_convert_None_to_Interface(r) for r in required])
order = len(required)
byorder = self._adapters
if order >= len(byorder):
Expand Down Expand Up @@ -210,7 +226,7 @@ def unregister(self, required, provided, name, value=None):
self.changed(self)

def subscribe(self, required, provided, value):
required = tuple(map(_convert_None_to_Interface, required))
required = tuple([_convert_None_to_Interface(r) for r in required])
name = u''
order = len(required)
byorder = self._subscribers
Expand All @@ -237,7 +253,7 @@ def subscribe(self, required, provided, value):
self.changed(self)

def unsubscribe(self, required, provided, value=None):
required = tuple(map(_convert_None_to_Interface, required))
required = tuple([_convert_None_to_Interface(r) for r in required])
order = len(required)
byorder = self._subscribers
if order >= len(byorder):
Expand Down Expand Up @@ -541,7 +557,7 @@ def _uncached_lookup(self, required, provided, name=u''):
return result

def queryMultiAdapter(self, objects, provided, name=u'', default=None):
factory = self.lookup(map(providedBy, objects), provided, name)
factory = self.lookup([providedBy(o) for o in objects], provided, name)
if factory is None:
return default

Expand Down Expand Up @@ -596,7 +612,7 @@ def _uncached_subscriptions(self, required, provided):
return result

def subscribers(self, objects, provided):
subscriptions = self.subscriptions(map(providedBy, objects), provided)
subscriptions = self.subscriptions([providedBy(o) for o in objects], provided)
if provided is None:
result = ()
for subscription in subscriptions:
Expand Down

0 comments on commit 32e25b3

Please sign in to comment.