- edited description
use deque() for event lists so that remove() raises during an event
Issue #3163
resolved
quick test:
import unittest
from sqlalchemy import event
from sqlalchemy.orm import Session
class Receiver(object):
def __init__(self, instance, remove):
self.instance = instance
self.remove = remove
def __call__(self, session, context):
self.instance.calls += 1
if self.remove:
event.remove(self.instance.session, 'after_flush_postexec', self)
@classmethod
def register(cls, instance, remove):
self = cls(instance, remove=remove)
event.listen(instance.session, 'after_flush_postexec', self)
class FlushingTest(unittest.TestCase):
def setUp(self):
self.session = Session()
self.calls = 0
def flush(self):
self.session.dispatch.after_flush_postexec(self.session, {})
def testTripleRemove(self):
Receiver.register(self, remove=True)
Receiver.register(self, remove=True)
Receiver.register(self, remove=True)
self.flush()
self.assertEqual(self.calls, 3)
unittest.main()
which fails, because we're iterating through a list.
This patch replaces the lists with a deque, just as fast or faster than a list, all tests pass:
diff --git a/lib/sqlalchemy/event/attr.py b/lib/sqlalchemy/event/attr.py
index 7641b59..dba1063 100644
--- a/lib/sqlalchemy/event/attr.py
+++ b/lib/sqlalchemy/event/attr.py
@@ -37,6 +37,7 @@ from . import registry
from . import legacy
from itertools import chain
import weakref
+import collections
class RefCollection(object):
@@ -96,8 +97,8 @@ class _DispatchDescriptor(RefCollection):
self.update_subclass(cls)
else:
if cls not in self._clslevel:
- self._clslevel[cls] = []
- self._clslevel[cls].insert(0, event_key._listen_fn)
+ self._clslevel[cls] = collections.deque()
+ self._clslevel[cls].appendleft(event_key._listen_fn)
registry._stored_in_collection(event_key, self)
def append(self, event_key, propagate):
@@ -113,13 +114,13 @@ class _DispatchDescriptor(RefCollection):
self.update_subclass(cls)
else:
if cls not in self._clslevel:
- self._clslevel[cls] = []
+ self._clslevel[cls] = collections.deque()
self._clslevel[cls].append(event_key._listen_fn)
registry._stored_in_collection(event_key, self)
def update_subclass(self, target):
if target not in self._clslevel:
- self._clslevel[target] = []
+ self._clslevel[target] = collections.deque()
clslevel = self._clslevel[target]
for cls in target.__mro__[1:]:
if cls in self._clslevel:
@@ -145,7 +146,7 @@ class _DispatchDescriptor(RefCollection):
to_clear = set()
for dispatcher in self._clslevel.values():
to_clear.update(dispatcher)
- dispatcher[:] = []
+ dispatcher.clear()
registry._clear(self, to_clear)
def for_modify(self, obj):
@@ -287,7 +288,7 @@ class _ListenerCollection(RefCollection, _CompoundListener):
self.parent_listeners = parent._clslevel[target_cls]
self.parent = parent
self.name = parent.__name__
- self.listeners = []
+ self.listeners = collections.deque()
self.propagate = set()
def for_modify(self, obj):
@@ -337,7 +338,7 @@ class _ListenerCollection(RefCollection, _CompoundListener):
def clear(self):
registry._clear(self, self.listeners)
self.propagate.clear()
- self.listeners[:] = []
+ self.listeners.clear()
class _JoinedDispatchDescriptor(object):
diff --git a/lib/sqlalchemy/event/registry.py b/lib/sqlalchemy/event/registry.py
index a34de3c..ba2f671 100644
--- a/lib/sqlalchemy/event/registry.py
+++ b/lib/sqlalchemy/event/registry.py
@@ -243,4 +243,4 @@ class _EventKey(object):
def prepend_to_list(self, owner, list_):
_stored_in_collection(self, owner)
- list_.insert(0, self._listen_fn)
+ list_.appendleft(self._listen_fn)
the deque(), nicely enough, raises!
RuntimeError: deque mutated during iteration
this is a totally easy win.
Comments (2)
-
reporter -
reporter - changed status to resolved
- Removing (or adding) an event listener at the same time that the event
is being run itself, either from inside the listener or from a
concurrent thread, now raises a RuntimeError, as the collection used is
now an instance of
colletions.deque()
and does not support changes while being iterated. Previously, a plain Python list was used where removal from inside the event itself would produce silent failures. fixes#3163
→ <<cset 4a4cccfee5a2>>
- Log in to comment