The python _p_activate does state management like the C version does: set to...

The python _p_activate does state management like the C version does: set to changed during jar.setstate to prevent objects that set attributes in their __setstate__ from being registered with the jar, and unconditionally mark the object as saved when finished. This fixes problems with BTrees and DB churn, among other things. Test this.

persistent/persistence.py

@@ -345,7 +345,8 @@ class Persistent(object):
         oga = _OGA
         before = oga(self, '_Persistent__flags')
         if before is None: # Only do this if we're a ghost
-            _OSA(self, '_Persistent__flags', 0) # up-to-date
+            # Begin by marking up-to-date in case we bail early
+            _OSA(self, '_Persistent__flags', 0)
             jar = oga(self, '_Persistent__jar')
             if jar is None:
                 return
@@ -353,11 +354,24 @@ class Persistent(object):
             if oid is None:
                 return
 
+            # If we're actually going to execute a set-state,
+            # mark as changed to prevent any recursive call
+            # (actually, our earlier check that we're a ghost should
+            # prevent this, but the C implementation sets it to changed
+            # while calling jar.setstate, and this is observable to clients).
+            # The main point of this is to prevent changes made during
+            # setstate from registering the object with the jar.
+            _OSA(self, '_Persistent__flags', CHANGED)
             try:
                 jar.setstate(self)
             except:
                 _OSA(self, '_Persistent__flags', before)
                 raise
+            else:
+                # If we succeed, no matter what the implementation
+                # of setstate did, mark ourself as up-to-date. The
+                # C implementation unconditionally does this.
+                _OSA(self, '_Persistent__flags', 0) # up-to-date
 
     # In the C implementation, _p_invalidate winds up calling
     # _p_deactivate. There are ZODB tests that depend on this;

persistent/tests/test_persistence.py

@@ -19,9 +19,18 @@ import sys
 py_impl = getattr(platform, 'python_implementation', lambda: None)
 _is_pypy3 = py_impl() == 'PyPy' and sys.version_info[0] > 2
 
+#pylint: disable=R0904,W0212,E1101
 
 class _Persistent_Base(object):
 
+    def _getTargetClass(self):
+        """Return the type of the persistent object to test"""
+        raise NotImplementedError()
+
+    def _makeCache(self, jar):
+        """Return a new pickle cache"""
+        raise NotImplementedError()
+
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
@@ -31,11 +40,17 @@ class _Persistent_Base(object):
 
         @implementer(IPersistentDataManager)
         class _Jar(object):
+            _cache = None
+            # Set this to a value to have our `setstate`
+            # pass it through to the object's __setstate__
+            setstate_calls_object = None
             def __init__(self):
                 self._loaded = []
                 self._registered = []
             def setstate(self, obj):
                 self._loaded.append(obj._p_oid)
+                if self.setstate_calls_object is not None:
+                    obj.__setstate__(self.setstate_calls_object)
             def register(self, obj):
                 self._registered.append(obj._p_oid)
 
@@ -1073,6 +1088,32 @@ class _Persistent_Base(object):
         inst._p_activate()
         self.assertEqual(list(jar._loaded), [OID])
 
+    def test__p_activate_leaves_object_in_saved_even_if_object_mutated_self(self):
+        # If the object's __setstate__ set's attributes
+        # when called by p_activate, the state is still
+        # 'saved' when done. Furthemore, the object is not
+        # registered with the jar
+
+        class WithSetstate(self._getTargetClass()):
+            state = None
+            def __setstate__(self, state):
+                self.state = state
+
+        inst, jar, OID = self._makeOneWithJar(klass=WithSetstate)
+        inst._p_invalidate() # make it a ghost
+        self.assertEqual(inst._p_status, 'ghost')
+
+        jar.setstate_calls_object = 42
+        inst._p_activate()
+        # It get loaded
+        self.assertEqual(list(jar._loaded), [OID])
+        # and __setstate__ got called to mutate the object
+        self.assertEqual(inst.state, 42)
+        # but it's still in the saved state
+        self.assertEqual(inst._p_status, 'saved')
+        # and it is not registered as changed by the jar
+        self.assertEqual(list(jar._registered), [])
+
     def test__p_deactivate_from_unsaved(self):
         inst = self._makeOne()
         inst._p_deactivate()