Skip to content

RuntimeError: dictionary changed size during iteration when committing session #1891

Closed
@sqlalchemy-bot

Description

@sqlalchemy-bot

Migrated issue, originally created by Anonymous

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

Activity

sqlalchemy-bot

sqlalchemy-bot commented on Aug 25, 2010

@sqlalchemy-bot
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

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()
sqlalchemy-bot

sqlalchemy-bot commented on Aug 25, 2010

@sqlalchemy-bot
CollaboratorAuthor

Changes by Michael Bayer (@zzzeek):

  • removed labels: access
  • added labels: orm
  • set milestone to "0.6.4"
  • set assignee to "zzzeek"
sqlalchemy-bot

sqlalchemy-bot commented on Aug 25, 2010

@sqlalchemy-bot
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

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()
sqlalchemy-bot

sqlalchemy-bot commented on Aug 25, 2010

@sqlalchemy-bot
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

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
sqlalchemy-bot

sqlalchemy-bot commented on Sep 7, 2010

@sqlalchemy-bot
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

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

sqlalchemy-bot

sqlalchemy-bot commented on Sep 7, 2010

@sqlalchemy-bot
CollaboratorAuthor

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
added this to the 0.6.4 milestone on Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingorm

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @sqlalchemy-bot

        Issue actions

          RuntimeError: dictionary changed size during iteration when committing session · Issue #1891 · sqlalchemy/sqlalchemy