- some changes to the identity map regarding

    rare weakref callbacks during iterations.
    The mutex has been removed as it apparently
    can cause a reentrant (i.e. in one thread) deadlock,
    perhaps when gc collects objects at the point of
    iteration in order to gain more memory.  It is hoped
    that "dictionary changed during iteration" will
    be exceedingly rare as iteration methods internally
    acquire the full list of objects in a single values()
    call. [ticket:2087]

diff --git a/CHANGES b/CHANGES
index b6b6b3bf8e9a0a309afd54bc007ca36660fc9137..202e34434e7ca873bea37fae032996909ef2710f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -5,6 +5,18 @@ CHANGES
 =======
 0.7.0b3
 =======
+- orm
+  - some changes to the identity map regarding
+    rare weakref callbacks during iterations.
+    The mutex has been removed as it apparently 
+    can cause a reentrant (i.e. in one thread) deadlock,
+    perhaps when gc collects objects at the point of 
+    iteration in order to gain more memory.  It is hoped
+    that "dictionary changed during iteration" will
+    be exceedingly rare as iteration methods internally
+    acquire the full list of objects in a single values() 
+    call. [ticket:2087]
+
 - sql
   - Added a fully descriptive error message for the
     case where Column is subclassed and _make_proxy()

diff --git a/lib/sqlalchemy/orm/identity.py b/lib/sqlalchemy/orm/identity.py
index b3a7f8bc3fb79ca7bf727d961ee5658f1eb8c364..a53d9f52db23a8bce17fb6044b44dfc7a8e66154 100644 (file)
--- a/lib/sqlalchemy/orm/identity.py
+++ b/lib/sqlalchemy/orm/identity.py
@@ -5,8 +5,6 @@
 # the MIT License: http://www.opensource.org/licenses/mit-license.php
 
 import weakref
-
-from sqlalchemy import util as base_util
 from sqlalchemy.orm import attributes
 
 
@@ -22,9 +20,6 @@ class IdentityMap(dict):
     def add(self, state):
         raise NotImplementedError()
 
-    def remove(self, state):
-        raise NotImplementedError()
-
     def update(self, dict):
         raise NotImplementedError("IdentityMap uses add() to insert data")
 
@@ -83,7 +78,6 @@ class IdentityMap(dict):
 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)
@@ -143,27 +137,6 @@ class WeakInstanceDict(IdentityMap):
         dict.__setitem__(self, key, state)
         self._manage_incoming_state(state)
 
-    def remove_key(self, key):
-        state = dict.__getitem__(self, key)
-        self.remove(state)
-
-    def remove(self, 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):
-        if self.contains_state(state):
-            dict.__delitem__(self, state.key)
-            self._manage_removed_state(state)
-
     def get(self, key, default=None):
         if not dict.__contains__(self, key):
             return default
@@ -175,53 +148,53 @@ class WeakInstanceDict(IdentityMap):
                 return default
         return o
 
-    def items(self):
+    def _items(self):
+        values = self.all_states()
+        result = []
+        for state in values:
+            value = state.obj()
+            if value is not None:
+                result.append((state.key, value))
+        return result
+
+    def _values(self):
+        values = self.all_states()
+        result = []
+        for state in values:
+            value = state.obj()
+            if value is not None:
+                result.append(value)
+
+        return result
+
+    # Py3K
+    #def items(self):
+    #    return iter(self._items())
+    #
+    #def values(self):
+    #    return iter(self._values())
     # Py2K
-        return list(self.iteritems())
-
+    items = _items
     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))
-
-            return iter(result)
-        finally:
-            self._remove_mutex.release()
-
-    def values(self):
-    # Py2K
-        return list(self.itervalues())
+        return iter(self.items())
 
+    values = _values
     def itervalues(self):
+        return iter(self.values())
     # end Py2K
-        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):
-        self._remove_mutex.acquire()
-        try:
-            # Py3K
-            # return list(dict.values(self))
+        # 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 discard(self, state):
+        st = dict.get(self, state.key, None)
+        if st is state:
+            dict.__delitem__(self, state.key)
+            self._manage_removed_state(state)
 
     def prune(self):
         return 0
@@ -256,21 +229,13 @@ class StrongInstanceDict(IdentityMap):
             dict.__setitem__(self, state.key, state.obj())
             self._manage_incoming_state(state)
 
-    def remove(self, state):
-        if attributes.instance_state(dict.pop(self, state.key)) \
-            is not state:
-            raise AssertionError('State %s is not present in this '
-                                 'identity map' % state)
-        self._manage_removed_state(state)
-
     def discard(self, state):
-        if self.contains_state(state):
-            dict.__delitem__(self, state.key)
-            self._manage_removed_state(state)
-
-    def remove_key(self, key):
-        state = attributes.instance_state(dict.__getitem__(self, key))
-        self.remove(state)
+        obj = dict.get(self, state.key, None)
+        if obj is not None:
+            st = attributes.instance_state(obj)
+            if st is state:
+                dict.__delitem__(self, state.key)
+                self._manage_removed_state(state)
 
     def prune(self):
         """prune unreferenced, non-dirty states."""

diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index e5e74eafb4601f73676693b4dbfd00d101a35c54..e29c5c03ad1ee37845faad4331c45ffc01a40ee0 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -68,10 +68,7 @@ class InstanceState(object):
     def _cleanup(self, ref):
         instance_dict = self._instance_dict()
         if instance_dict:
-            try:
-                instance_dict.remove(self)
-            except AssertionError:
-                pass
+            instance_dict.discard(self)
 
         self.callables = {}
         self.session_id = None
@@ -516,10 +513,7 @@ class MutableAttrInstanceState(InstanceState):
         else:
             instance_dict = self._instance_dict()
             if instance_dict:
-                try:
-                    instance_dict.remove(self)
-                except AssertionError:
-                    pass
+                instance_dict.discard(self)
             self.dispose()
 
     def __resurrect(self):

diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index f0a925541088857ce513d5e778a2defae1d5540e..9ef00e6472bf493a12c7f7b1d3bbe7f2298a0d3a 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -1280,7 +1280,7 @@ class DisposedStates(_base.MappedTest):
         sess.identity_map.all_states = lambda : all_states
         for obj in objs:
             state = attributes.instance_state(obj)
-            sess.identity_map.remove(state)
+            sess.identity_map.discard(state)
             state.dispose()
 
     def _test_session(self, **kwargs):