1. Michael Bayer
  2. sqlalchemy

Issues

Issue #3163 resolved

use deque() for event lists so that remove() raises during an event

Michael Bayer
repo owner created an issue

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)

  1. Michael Bayer reporter
    • 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>>

  2. Log in to comment