On deactivate release in-slots objects too

On ._p_deactivate() and ._p_invalidate(), when an object goes to ghost
state, objects referenced by all its attributes, except related to
persistence machinery, are released, this way freeing memory (if they
were referenced only from going-to-ghost object).

That's the idea - an object in ghost state is simply a stub, which loads
its content on first access (via hooking into get/set attr) while
occupying minimal memory in not-yet-loaded state.

However the above is not completely true right now, as currently on
ghostification only object's .__dict__ is released, while in-slots objects
are retained attached to ghost object staying in RAM:

    ---- 8< ----
    from ZODB import DB
    from persistent import Persistent
    import gc

    db = DB(None)
    jar = db.open()

    class C:
        def __init__(self, v):
            self.v = v
        def __del__(self):
            print 'released (%s)' % self.v

    class P1(Persistent):
        pass

    class P2(Persistent):
        __slots__ = ('aaa')

    p1 = P1()
    jar.add(p1)
    p1.aaa = C(1)

    p2 = P2()
    jar.add(p2)
    p2.aaa = C(2)

    p1._p_invalidate()
    # "released (1)" is printed

    p2._p_invalidate()
    gc.collect()
    # "released (2)" is NOT printed     <--
    ---- 8< ----

So teach ghostify() & friends to release objects in slots to free-up
memory when an object goes to ghost state.

NOTE PyErr_Occurred() added after ghostify() calls because
pickle_slotnames() can raise an error, but we do not want to change
ghostify() prototype for backward compatibility reason - as it is used
in cPersistenceCAPIstruct.

( I hit this bug with wendelin.core which uses proxies to load
  data from DB to virtual memory manager and then deactivate proxy right
  after load has been completed:

  https://lab.nexedi.com/nexedi/wendelin.core/blob/f7803634/bigfile/file_zodb.py#L239
  https://lab.nexedi.com/nexedi/wendelin.core/blob/f7803634/bigfile/file_zodb.py#L295 )

persistent/cPersistence.c

@@ -82,6 +82,7 @@ fatal_1350(cPersistentObject *self, const char *caller, const char *detail)
 #endif
 
 static void ghostify(cPersistentObject*);
+static PyObject * pickle_slotnames(PyTypeObject *cls);
 
 static PyObject * convert_name(PyObject *name);
 
@@ -150,7 +151,7 @@ accessed(cPersistentObject *self)
 static void
 ghostify(cPersistentObject *self)
 {
-    PyObject **dictptr;
+    PyObject **dictptr, *slotnames;
 
     /* are we already a ghost? */
     if (self->state == cPersistent_GHOST_STATE)
@@ -180,6 +181,8 @@ ghostify(cPersistentObject *self)
         _estimated_size_in_bytes(self->estimated_size);
     ring_del(&self->ring);
     self->state = cPersistent_GHOST_STATE;
+
+    /* clear __dict__ */
     dictptr = _PyObject_GetDictPtr((PyObject *)self);
     if (dictptr && *dictptr)
     {
@@ -187,6 +190,48 @@ ghostify(cPersistentObject *self)
         *dictptr = NULL;
     }
 
+    /* clear all slots besides _p_* */
+    slotnames = pickle_slotnames(Py_TYPE(self));
+    if (slotnames && slotnames != Py_None)
+    {
+        int i;
+
+        for (i = 0; i < PyList_GET_SIZE(slotnames); i++)
+        {
+            PyObject *name;
+            char *cname;
+            int is_special;
+
+            name = PyList_GET_ITEM(slotnames, i);
+#ifdef PY3K
+            if (PyUnicode_Check(name))
+            {
+                PyObject *converted = convert_name(name);
+                cname = PyBytes_AS_STRING(converted);
+#else
+            if (PyBytes_Check(name))
+            {
+                cname = PyBytes_AS_STRING(name);
+#endif
+                is_special = !strncmp(cname, "_p_", 3);
+#ifdef PY3K
+                Py_DECREF(converted);
+#endif
+                if (is_special) /* skip persistent */
+                {
+                    continue;
+                }
+            }
+
+            /* NOTE: this skips our delattr hook */
+            if (PyObject_GenericSetAttr((PyObject *)self, name, NULL) < 0)
+                /* delattr of non-set slot will raise AttributeError - we
+                 * simply ignore. */
+                PyErr_Clear();
+        }
+    }
+    Py_XDECREF(slotnames);
+
     /* We remove the reference to the just ghosted object that the ring
     * holds.  Note that the dictionary of oids->objects has an uncounted
     * reference, so if the ring's reference was the only one, this frees
@@ -270,6 +315,8 @@ Per__p_deactivate(cPersistentObject *self)
             called directly. Methods that override this need to
             do the same! */
         ghostify(self);
+        if (PyErr_Occurred())
+            return NULL;
     }
 
     Py_INCREF(Py_None);
@@ -298,6 +345,8 @@ Per__p_invalidate(cPersistentObject *self)
         if (Per_set_changed(self, NULL) < 0)
             return NULL;
         ghostify(self);
+        if (PyErr_Occurred())
+            return NULL;
     }
     Py_INCREF(Py_None);
     return Py_None;

persistent/persistence.py

@@ -303,11 +303,11 @@ class Persistent(object):
                     _OGA(self, '_p_register')()
         object.__delattr__(self, name)
 
-    def _slotnames(self):
+    def _slotnames(self, _v_exclude=True):
         slotnames = copy_reg._slotnames(type(self))
         return [x for x in slotnames
                    if not x.startswith('_p_') and
-                      not x.startswith('_v_') and
+                      not (x.startswith('_v_') and _v_exclude) and
                       not x.startswith('_Persistent__') and
                       x not in Persistent.__slots__]
 
@@ -423,6 +423,10 @@ class Persistent(object):
         idict = getattr(self, '__dict__', None)
         if idict is not None:
             idict.clear()
+        type_ = type(self)
+        for slotname in Persistent._slotnames(self, _v_exclude=False):
+            getattr(type_, slotname).__delete__(self)
+
         # Implementation detail: deactivating/invalidating
         # updates the size of the cache (if we have one)
         # by telling it this object no longer takes any bytes

persistent/tests/test_persistence.py

@@ -1319,6 +1319,27 @@ class _Persistent_Base(object):
         self.assertEqual(list(jar._loaded), [])
         self.assertEqual(list(jar._registered), [])
 
+    def test__p_invalidate_from_changed_w_slots(self):
+        class Derived(self._getTargetClass()):
+            __slots__ = ('myattr1', 'myattr2')
+            def __init__(self):
+                self.myattr1 = 'value1'
+                self.myattr2 = 'value2'
+        inst, jar, OID = self._makeOneWithJar(Derived)
+        inst._p_activate()
+        inst._p_changed = True
+        jar._loaded = []
+        jar._registered = []
+        self.assertEqual(Derived.myattr1.__get__(inst), 'value1')
+        self.assertEqual(Derived.myattr2.__get__(inst), 'value2')
+        inst._p_invalidate()
+        self.assertEqual(inst._p_status, 'ghost')
+        self.assertEqual(list(jar._loaded), [])
+        self.assertRaises(AttributeError, lambda: Derived.myattr1.__get__(inst))
+        self.assertRaises(AttributeError, lambda: Derived.myattr2.__get__(inst))
+        self.assertEqual(list(jar._loaded), [])
+        self.assertEqual(list(jar._registered), [])
+
     def test__p_invalidate_from_sticky(self):
         inst, jar, OID = self._makeOneWithJar()
         inst._p_activate() # XXX