From 381da62d31235fef603ee070970a0f7decfa9b25 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 7 May 2011 10:44:49 -0400 Subject: [PATCH] - Backported 0.7's identity map implementation, which does not use a mutex around removal. This as some users were still getting deadlocks despite the adjustments in 0.6.7; the 0.7 approach that doesn't use a mutex does not appear to produce "dictionary changed size" issues, the original rationale for the mutex. [ticket:2148] --- CHANGES | 8 +++ lib/sqlalchemy/orm/identity.py | 106 +++++++++++++++------------------ 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/CHANGES b/CHANGES index c632465a29..4cc61e0577 100644 --- a/CHANGES +++ b/CHANGES @@ -17,6 +17,14 @@ CHANGES identity-map compatible with that of the primary mapper [ticket:2151] + - Backported 0.7's identity map implementation, which + does not use a mutex around removal. This as some users + were still getting deadlocks despite the adjustments + in 0.6.7; the 0.7 approach that doesn't use a mutex + does not appear to produce "dictionary changed size" + issues, the original rationale for the mutex. + [ticket:2148] + - sql - Fixed bug whereby if FetchedValue was passed to column server_onupdate, it would not diff --git a/lib/sqlalchemy/orm/identity.py b/lib/sqlalchemy/orm/identity.py index 5df405fb5b..bc4444a2ea 100644 --- 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,7 +20,7 @@ class IdentityMap(dict): def add(self, state): raise NotImplementedError() - def remove(self, state): + def discard(self, state): raise NotImplementedError() def update(self, dict): @@ -31,6 +29,10 @@ class IdentityMap(dict): def clear(self): raise NotImplementedError("IdentityMap uses remove() to remove data") + def remove(self, state): + # deprecated, here for 0.6 compat + return self.discard(state) + def _manage_incoming_state(self, state): state._instance_dict = self._wr @@ -83,7 +85,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) @@ -123,35 +124,25 @@ class WeakInstanceDict(IdentityMap): self._manage_incoming_state(state) def add(self, state): - if state.key in self: - if dict.__getitem__(self, state.key) is not state: - raise AssertionError("A conflicting state is already " - "present in the identity map for key %r" - % (state.key, )) - else: - dict.__setitem__(self, state.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) + key = state.key + # inline of self.__contains__ + if dict.__contains__(self, key): + try: + existing_state = dict.__getitem__(self, key) + if existing_state is not state: + o = existing_state.obj() + if o is None: + o = existing_state._is_really_none() + if o is not None: + raise AssertionError("A conflicting state is already " + "present in the identity map for key %r" + % (key, )) + else: + return + except KeyError: + pass + dict.__setitem__(self, key, state) + self._manage_incoming_state(state) def get(self, key, default=None): state = dict.get(self, key, default) @@ -160,8 +151,8 @@ class WeakInstanceDict(IdentityMap): o = state.obj() if o is None: o = state._is_really_none() - if o is None: - return default + if o is None: + return default return o def _items(self): @@ -200,15 +191,17 @@ class WeakInstanceDict(IdentityMap): # end Py2K def all_states(self): - self._remove_mutex.acquire() - try: - # Py3K - # return list(dict.values(self)) - # Py2K - return dict.values(self) - # end Py2K - finally: - self._remove_mutex.release() + # Py3K + # return list(dict.values(self)) + # Py2K + return dict.values(self) + # end Py2K + + 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 @@ -234,25 +227,22 @@ class StrongInstanceDict(IdentityMap): def add(self, state): if state.key in self: - if attributes.instance_state(dict.__getitem__(self, state.key)) is not state: - raise AssertionError("A conflicting state is already present in the identity map for key %r" % (state.key, )) + if attributes.instance_state(dict.__getitem__(self, + state.key)) is not state: + raise AssertionError('A conflicting state is already ' + 'present in the identity map for key %r' + % (state.key, )) else: 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.""" -- 2.47.3