Changed the strategy for handling invalidation of classes.

No-longer use setklassstate.  Instead, just call _p_invalidate, as
with any other object.  This changes didn't break any tests, so I
assume that this was untested. :(

Changed the strategy for invalidating objects.  Non-ghostifiable
objects will load their state when they are invalidated.  We have to
worry about other invalidations that come in while this is happening.
See the comment in _flush_invalidations.

We need to force all invalidations to take this into account, by going
through _flush_invalidations.  I haven't done this yet, but I have
left some XXX comments in places where it needs to be done to remind
myself that this needs to be done.

lib/python/ZODB/Connection.py

@@ -141,8 +141,7 @@ class Connection(ExportImport, object):
         tpc_finish, tpc_abort, sortKey, abort, commit, commit_sub,
         abort_sub
       - `Database Invalidation Methods`: invalidate, _setDB
-      - `IPersistentDataManager Methods`: setstate, register,
-        setklassstate
+      - `IPersistentDataManager Methods`: setstate, register
       - `Other Methods`: oldstate, exchange, getDebugInfo, setDebugInfo,
         getTransferCounts
 
@@ -219,15 +218,17 @@ class Connection(ExportImport, object):
         # from a single transaction should be applied atomically, so
         # the lock must be held when reading _invalidated.
 
-        # XXX It sucks that we have to hold the lock to read
+        # It sucks that we have to hold the lock to read
         # _invalidated.  Normally, _invalidated is written by calling
         # dict.update, which will execute atomically by virtue of the
         # GIL.  But some storage might generate oids where hash or
         # compare invokes Python code.  In that case, the GIL can't
         # save us.
+
         self._inv_lock = threading.Lock()
         self._invalidated = d = {}
         self._invalid = d.has_key
+        
 
         # We intend to prevent committing a transaction in which
         # ReadConflictError occurs.  _conflicts is the set of oids that
@@ -429,6 +430,9 @@ class Connection(ExportImport, object):
         # A Connection is only ever associated with a single DB
         # and Storage.
 
+        # YYY This is a holdover from Python 1.  It is trying to break
+        # cycles
+
         self._db = odb
         self._storage = odb._storage
         self._sortKey = odb._storage.sortKey
@@ -473,6 +477,14 @@ class Connection(ExportImport, object):
                 del obj._p_jar
                 del obj._p_oid
             else:
+                
+                # XXX Need to change strategy to acount for
+                # non-ghostifiables that need to load right away.
+                # We need to simply call self.invalidate and let
+                # _flush_invalidations actually perform the
+                # invalidations. I'd do this now if I had a test for
+                # it. :)                
+
                 self._cache.invalidate(oid)
 
         self._tpc_cleanup()
@@ -683,6 +695,13 @@ class Connection(ExportImport, object):
         self._storage = self._tmp
         self._tmp = None
 
+                
+        # XXX Need to change strategy to acount for non-ghostifiables
+        # that need to load right away.  We need to simply call
+        # self.invalidate and let _flush_invalidations actually
+        # perform the invalidations. I'd do this now if I had a test
+        # for it. :)
+
         self._cache.invalidate(src._index.keys())
         self._invalidate_creating(src._creating)
 
@@ -747,11 +766,43 @@ class Connection(ExportImport, object):
     def _flush_invalidations(self):
         self._inv_lock.acquire()
         try:
-            self._cache.invalidate(self._invalidated)
-            self._invalidated.clear()
+            
+            # Non-ghostifiable objects may need to read when they are
+            # invalidated, so, we'll quickly just replace the
+            # invalidating dict with a new one.  We'll then process
+            # the invalidations after freeing the lock *and* after
+            # reseting the time.  This means that invalidations will
+            # happen after the start of the transactions.  They are
+            # subject to conflict errors and to reading old data,
+
+            # XXX There is a potential problem lurking for persistent
+            # classes.  Suppose we have an invlidation of a persistent
+            # class and of an instance.  If the instance is
+            # invalidated first and if the invalidation logic uses
+            # data read from the class, then the invalidation could
+            # be performed with state data.  Or, suppose that there
+            # are instances of the class that are freed as a result of
+            # invalidating some object.  Perhaps code in their __del__
+            # uses class data.  Really, the only way to properly fix
+            # this is to, in fact, make classes ghostifiable.  Then
+            # we'd have to reimplement attribute lookup to check the
+            # class state and, if necessary, activate the class.  It's
+            # much worse than that though, because we'd also need to
+            # deal with slots.  When a class is ghostified, we'd need
+            # to replace all of the slot operations with versions that
+            # reloaded the object when caled. It's hard to say which
+            # is better for worse.  For now, it seems the risk of
+            # using a class while objects are being invalidated seems
+            # small enough t be acceptible.
+
+            invalidated = self._invalidated
+            self._invalidated = {}
             self._txn_time = None
         finally:
             self._inv_lock.release()
+
+        self._cache.invalidate(invalidated)
+
         # Now is a good time to collect some garbage
         self._cache.incrgc()
 
@@ -841,7 +892,9 @@ class Connection(ExportImport, object):
         # because we have to check again after the load anyway.
 
         if (obj._p_oid in self._invalidated
-            and not myhasattr(obj, "_p_independent")):
+            and not myhasattr(obj, "_p_independent")
+            and not self._invalidated
+            ):
             # If the object has _p_independent(), we will handle it below.
             self._load_before_or_conflict(obj)
             return
@@ -939,27 +992,6 @@ class Connection(ExportImport, object):
         p = self._storage.loadSerial(obj._p_oid, tid)
         return self._reader.getState(p)
 
-    def setklassstate(self, obj):
-        # Special case code to handle ZClasses, I think.
-        # Called the cache when an object of type type is invalidated.
-        try:
-            oid = obj._p_oid
-            p, serial = self._storage.load(oid, self._version)
-
-            # We call getGhost(), but we actually get a non-ghost back.
-            # The object is a class, which can't actually be ghosted.
-            copy = self._reader.getGhost(p)
-            obj.__dict__.clear()
-            obj.__dict__.update(copy.__dict__)
-
-            obj._p_oid = oid
-            obj._p_jar = self
-            obj._p_changed = 0
-            obj._p_serial = serial
-        except:
-            self._log.error("setklassstate failed", exc_info=sys.exc_info())
-            raise
-
     def tpc_begin(self, transaction, sub=False):
         self._modified = []
 
@@ -1049,6 +1081,13 @@ class Connection(ExportImport, object):
         if self._import:
             self._import = None
         self._storage.tpc_abort(transaction)
+                
+        # XXX Need to change strategy to acount for non-ghostifiables
+        # that need to load right away.  We need to simply call
+        # self.invalidate and let _flush_invalidations actually
+        # perform the invalidations. I'd do this now if I had a test
+        # for it. :)
+
         self._cache.invalidate(self._modified)
         self._invalidate_creating()
         while self._added: