RuntimeError: dictionary changed size during iteration when committing session

[Former user]

With Python 2.6.5 and SQLAlchemy 0.6, we have been getting occasional RuntimeErrors. Relevant portion of the traceback is as follows:

File "source", line 189, in insert_or_update query.update(update_values) File "/usr/local/lib/python2.6/dist-packages/sqlalchemy/orm/query.py", line 1970, in update for (cls, pk),obj in session.identity_map.iteritems(): File "/usr/local/lib/python2.6/dist-packages/sqlalchemy/orm/identity.py", line 162, in iteritems for state in dict.itervalues(self): RuntimeError: dictionary changed size during iteration

[Mike Bayer]

here's a reproducer, though in your case its almost certainly async gc causing it:

!#python
from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
import gc

Base = declarative_base()
class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)

e = create_engine('sqlite://', echo=True)

Base.metadata.create_all(e)

session = sessionmaker(e)()

session.add_all([   A(), A(), A(), A()
](
))
session.commit()

a1, a2, a3, a4 = session.query(A).all()

for i, (key, value) in enumerate(session.identity_map.iteritems()):
    if i == 2:
        del a3
        # may need this, though seems to 
        # reproduce without it
        # gc.collect()

[Mike Bayer]

and here is a patch, but I need you to try it out for me, please...it would be very helpful to test this code out in a reproducing environment before committing.

diff -r 1402a608ebc28031ce11dec9ed6a90785f09294d lib/sqlalchemy/orm/identity.py
--- a/lib/sqlalchemy/orm/identity.py    Wed Aug 25 11:01:46 2010 -0400
+++ b/lib/sqlalchemy/orm/identity.py    Wed Aug 25 13:42:44 2010 -0400
@@ -15,7 +15,8 @@
         self._mutable_attrs = set()
         self._modified = set()
         self._wr = weakref.ref(self)
-
+        self.remove_mutex = base_util.threading.Lock()
+        
     def replace(self, state):
         raise NotImplementedError()

@@ -43,7 +44,7 @@
         del state._instance_dict
         self._mutable_attrs.discard(state)
         self._modified.discard(state)
-
+            
     def _dirty_states(self):
         return self._modified.union(s for s in self._mutable_attrs.copy()
                                     if s.modified)
@@ -58,7 +59,14 @@
                 if state.modified:
                     return True
         return False
-            
+        
+    def _safe_items(self):
+        self.remove_mutex.acquire()
+        try:
+            return list(self.iteritems())
+        finally:
+            self.remove_mutex.release()
+        
     def has_key(self, key):
         return key in self

diff -r 1402a608ebc28031ce11dec9ed6a90785f09294d lib/sqlalchemy/orm/query.py
--- a/lib/sqlalchemy/orm/query.py   Wed Aug 25 11:01:46 2010 -0400
+++ b/lib/sqlalchemy/orm/query.py   Wed Aug 25 13:42:44 2010 -0400
@@ -2045,7 +2045,7 @@
             #TODO: detect when the where clause is a trivial primary key match
             objs_to_expunge = [                                obj for (cls, pk),obj in
-                                session.identity_map.iteritems()
+                                session.identity_map._safe_items()
                                 if issubclass(cls, target_cls) and
                                 eval_condition(obj)](
)
             for obj in objs_to_expunge:
@@ -2181,7 +2181,7 @@
         if synchronize_session == 'evaluate':
             target_cls = self._mapper_zero().class_

-            for (cls, pk),obj in session.identity_map.iteritems():
+            for (cls, pk),obj in session.identity_map._safe_items():
                 evaluated_keys = value_evaluators.keys()

                 if issubclass(cls, target_cls) and eval_condition(obj):
diff -r 1402a608ebc28031ce11dec9ed6a90785f09294d lib/sqlalchemy/orm/state.py
--- a/lib/sqlalchemy/orm/state.py   Wed Aug 25 11:01:46 2010 -0400
+++ b/lib/sqlalchemy/orm/state.py   Wed Aug 25 13:42:44 2010 -0400
@@ -62,10 +62,14 @@
     def _cleanup(self, ref):
         instance_dict = self._instance_dict()
         if instance_dict:
+            instance_dict.remove_mutex.acquire()
             try:
-                instance_dict.remove(self)
-            except AssertionError:
-                pass
+                try:
+                    instance_dict.remove(self)
+                except AssertionError:
+                    pass
+            finally:
+                instance_dict.remove_mutex.release()
         # remove possible cycles
         self.__dict__.pop('callables', None)
         self.dispose()

[Mike Bayer]

I'll probably commit this revised version in any case. Needs tests, needs to be checked on 2to3 also since the items/values methods here were always thorny with py3k. Simpler though since we are giving up on having iterators in use.

diff -r 1402a608ebc28031ce11dec9ed6a90785f09294d lib/sqlalchemy/orm/identity.py
--- a/lib/sqlalchemy/orm/identity.py    Wed Aug 25 11:01:46 2010 -0400
+++ b/lib/sqlalchemy/orm/identity.py    Wed Aug 25 21:33:57 2010 -0400
@@ -15,7 +15,7 @@
         self._mutable_attrs = set()
         self._modified = set()
         self._wr = weakref.ref(self)
-
+        
     def replace(self, state):
         raise NotImplementedError()

@@ -61,7 +61,7 @@

     def has_key(self, key):
         return key in self
-        
+    
     def popitem(self):
         raise NotImplementedError("IdentityMap uses remove() to remove data")

@@ -81,6 +81,9 @@
         raise NotImplementedError("IdentityMap uses remove() to remove data")

 class WeakInstanceDict(IdentityMap):
+    def __init__(self):
+        IdentityMap.__init__(self)
+        self._remove_mutex = base_util.threading.Lock()

     def __getitem__(self, key):
         state = dict.__getitem__(self, key)
@@ -134,8 +137,13 @@
         self.remove(state)

     def remove(self, state):
-        if dict.pop(self, state.key) is not state:
-            raise AssertionError("State %s is not present in this identity map" % state)
+        self._remove_mutex.acquire()
+        try:
+            if dict.pop(self, state.key) is not state:
+                raise AssertionError("State %s is not present in this identity map" % state)
+        finally:
+            self._remove_mutex.release()
+            
         self._manage_removed_state(state)

     def discard(self, state):
@@ -153,43 +161,56 @@
         if o is None:
             return default
         return o
+
+
+    def items(self):
+    # Py2K
+        return list(self.iteritems())

-    # Py2K        
-    def items(self):
-        return list(self.iteritems())
+    def iteritems(self):
+    # end Py2K
+        self._remove_mutex.acquire()
+        try:
+            result = [           for state in dict.values(self):
+                value = state.obj()
+                if value is not None:
+                    result.append((state.key, value))

-    def iteritems(self):
-        for state in dict.itervalues(self):
-    # end Py2K
-    # Py3K
-    #def items(self):
-    #    for state in dict.values(self):
-            value = state.obj()
-            if value is not None:
-                yield state.key, value
-
+            return iter(result)
+        finally:
+            self._remove_mutex.release()
+        
+    def values(self):
     # Py2K
-    def values(self):
         return list(self.itervalues())

     def itervalues(self):
-        for state in dict.itervalues(self):
     # end Py2K
-    # Py3K
-    #def values(self):
-    #    for state in dict.values(self):
-            instance = state.obj()
-            if instance is not None:
-                yield instance
+        self._remove_mutex.acquire()
+        try:
+            result = [](]
+)
+            for state in dict.values(self):
+                value = state.obj()
+                if value is not None:
+                    result.append(value)

+            return iter(result)
+        finally:
+            self._remove_mutex.release()
+    
     def all_states(self):
-        # Py3K
-        # return list(dict.values(self))
+        self._remove_mutex.acquire()
+        try:
+            # Py3K
+            # return list(dict.values(self))

-        # Py2K
-        return dict.values(self)
-        # end Py2K
-    
+            # Py2K
+            return dict.values(self)
+            # end Py2K
+        finally:
+            self._remove_mutex.release()
+            
     def prune(self):
         return 0

[Mike Bayer]

OOOkkkkk, well that patch is in 177fdcb982f84b234c91c0f0ad61c088880118e6, thanks for the feedback ! ;)

[Mike Bayer]

Removing milestone: 0.6.4 (automated comment)