- 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

3 files changed, 63 insertions, 8 deletions

diff --git a/lib/sqlalchemy/event/api.py b/lib/sqlalchemy/event/api.py
index 270e95c9c..b3d79bcf4 100644
--- a/lib/sqlalchemy/event/api.py
+++ b/lib/sqlalchemy/event/api.py
@@ -58,6 +58,32 @@ def listen(target, identifier, fn, *args, **kw):
     .. versionadded:: 0.9.4 Added ``once=True`` to :func:`.event.listen`
        and :func:`.event.listens_for`.
 
+    .. note::
+
+        The :func:`.listen` function cannot be called at the same time
+        that the target event is being run.   This has implications
+        for thread safety, and also means an event cannot be added
+        from inside the listener function for itself.  The list of
+        events to be run are present inside of a mutable collection
+        that can't be changed during iteration.
+
+        Event registration and removal is not intended to be a "high
+        velocity" operation; it is a configurational operation.  For
+        systems that need to quickly associate and deassociate with
+        events at high scale, use a mutable structure that is handled
+        from inside of a single listener.
+
+        .. versionchanged:: 1.0.0 - a ``collections.deque()`` object is now
+           used as the container for the list of events, which explicitly
+           disallows collection mutation while the collection is being
+           iterated.
+
+    .. seealso::
+
+        :func:`.listens_for`
+
+        :func:`.remove`
+
     """
 
     _event_key(target, identifier, fn).listen(*args, **kw)
@@ -89,6 +115,10 @@ def listens_for(target, identifier, *args, **kw):
     .. versionadded:: 0.9.4 Added ``once=True`` to :func:`.event.listen`
        and :func:`.event.listens_for`.
 
+    .. seealso::
+
+        :func:`.listen` - general description of event listening
+
     """
     def decorate(fn):
         listen(target, identifier, fn, *args, **kw)
@@ -120,6 +150,30 @@ def remove(target, identifier, fn):
 
     .. versionadded:: 0.9.0
 
+    .. note::
+
+        The :func:`.remove` function cannot be called at the same time
+        that the target event is being run.   This has implications
+        for thread safety, and also means an event cannot be removed
+        from inside the listener function for itself.  The list of
+        events to be run are present inside of a mutable collection
+        that can't be changed during iteration.
+
+        Event registration and removal is not intended to be a "high
+        velocity" operation; it is a configurational operation.  For
+        systems that need to quickly associate and deassociate with
+        events at high scale, use a mutable structure that is handled
+        from inside of a single listener.
+
+        .. versionchanged:: 1.0.0 - a ``collections.deque()`` object is now
+           used as the container for the list of events, which explicitly
+           disallows collection mutation while the collection is being
+           iterated.
+
+    .. seealso::
+
+        :func:`.listen`
+
     """
     _event_key(target, identifier, fn).remove()
 
diff --git a/lib/sqlalchemy/event/attr.py b/lib/sqlalchemy/event/attr.py
index 7641b595a..dba1063cf 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 a34de3cd7..ba2f671a3 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)