- Fixed a series of potential race conditions in

Session whereby asynchronous GC could remove unmodified,
no longer referenced items from the session as they were
present in a list of items to be processed, typically
during session.expunge_all() and dependent methods.

diff --git a/CHANGES b/CHANGES
index 68171c925fe6aa6f635022b6133fb8921c3c7f71..13b9a979be2fba68bf1e5c0e1f7a16b0d4fefc76 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -22,7 +22,13 @@ CHANGES
       around by explicitly stating the column in each
       submapper as well but this is fairly unworkable
       and also impossible when using declarative [ticket:1111].
-    
+
+    - Fixed a series of potential race conditions in 
+      Session whereby asynchronous GC could remove unmodified,
+      no longer referenced items from the session as they were 
+      present in a list of items to be processed, typically 
+      during session.expunge_all() and dependent methods.
+      
 - mysql
     - Quoting of MSEnum values for use in CREATE TABLE is now
       optional & will be quoted on demand as required.  (Quoting was

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index f883ebc98e9179b9ae40f505dc9a01c214191439..289294f4ce4f2daff12a4c2bf64885c8bf81cdb0 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -774,7 +774,13 @@ class Session(object):
 
         """
         for state in self.identity_map.all_states() + list(self._new):
-            del state.session_id
+            try:
+                del state.session_id
+            except AttributeError:
+                # asynchronous GC can sometimes result
+                # in the auto-disposal of an InstanceState within this process
+                # see test/orm/session.py DisposedStates
+                pass
 
         self.identity_map = self._identity_cls()
         self._new = {}

diff --git a/test/orm/session.py b/test/orm/session.py
index df9536650b5e99dd58c1ca191d996e8168b94bfe..9d3ab661064c891638f9eb5e8a7c7c54529b2ddd 100644 (file)
--- a/test/orm/session.py
+++ b/test/orm/session.py
@@ -2,7 +2,7 @@ import testenv; testenv.configure_for_tests()
 import gc
 import inspect
 import pickle
-from sqlalchemy.orm import create_session, sessionmaker
+from sqlalchemy.orm import create_session, sessionmaker, attributes
 from testlib import engines, sa, testing, config
 from testlib.sa import Table, Column, Integer, String
 from testlib.sa.orm import mapper, relation, backref
@@ -990,7 +990,87 @@ class SessionTest(_fixtures.FixtureTest):
         assert b in sess
         assert len(list(sess)) == 1
 
+class DisposedStates(testing.ORMTest):
+    keep_mappers = True
+    keep_tables = True
+    
+    def define_tables(self, metadata):
+        global t1
+        t1 = Table('t1', metadata, 
+            Column('id', Integer, primary_key=True),
+            Column('data', String(50))
+            )
+
+    def setup_mappers(self):
+        global T
+        class T(object):
+            def __init__(self, data):
+                self.data = data
+        mapper(T, t1)
+    
+    def tearDown(self):
+        from sqlalchemy.orm.session import _sessions
+        _sessions.clear()
+        super(DisposedStates, self).tearDown()
+        
+    def _set_imap_in_disposal(self, sess, *objs):
+        """remove selected objects from the given session, as though they 
+        were dereferenced and removed from WeakIdentityMap.
+        
+        Hardcodes the identity map's "all_states()" method to return the full list
+        of states.  This simulates the all_states() method returning results, afterwhich
+        some of the states get garbage collected (this normally only happens during
+        asynchronous gc).  The Session now has one or more 
+        InstanceState's which have been removed from the identity map and disposed.
+        
+        Will the Session not trip over this ???  Stay tuned.
+        
+        """
+        all_states = sess.identity_map.all_states()
+        sess.identity_map.all_states = lambda: all_states
+        for obj in objs:
+            state = attributes.instance_state(obj)
+            sess.identity_map.remove(state)
+            state.dispose()
+    
+    def _test_session(self, **kwargs):
+        global sess
+        sess = create_session(**kwargs)
+
+        data = o1, o2, o3, o4, o5 = [T('t1'), T('t2'), T('t3'), T('t4'), T('t5')]
+
+        sess.add_all(data)
+
+        sess.flush()
 
+        o1.data = 't1modified'
+        o5.data = 't5modified'
+        
+        self._set_imap_in_disposal(sess, o2, o4, o5)
+        return sess
+        
+    def test_flush(self):
+        self._test_session().flush()
+    
+    def test_clear(self):
+        self._test_session().clear()
+    
+    def test_close(self):
+        self._test_session().close()
+        
+    def test_expunge_all(self):
+        self._test_session().expunge_all()
+        
+    def test_expire_all(self):
+        self._test_session().expire_all()
+    
+    def test_rollback(self):
+        sess = self._test_session(autocommit=False, autoexpire=True)
+        sess.commit()
+        
+        sess.rollback()
+        
+        
 class SessionInterface(testing.TestBase):
     """Bogus args to Session methods produce actionable exceptions."""