On deactivate release in-slots objects only for classes that don't override __new__

Commit fe2219f4 taught Persistent to release in-slots objects on
deactivation. That however broke pure-python implementation of many
things because they were using __slots__ as a place which survive
deactivation.

As per discussion in

    https://github.com/zopefoundation/persistent/pull/44

and resolution:

    https://github.com/zopefoundation/persistent/pull/44#issuecomment-261019084

let's try to preserve backward compatibility by not releasing slots for
classes that override __new__

/proposed-by-and-helped @jimfulton

persistent/cPersistence.c

@@ -190,47 +190,52 @@ ghostify(cPersistentObject *self)
         *dictptr = NULL;
     }
 
-    /* clear all slots besides _p_* */
-    slotnames = pickle_slotnames(Py_TYPE(self));
-    if (slotnames && slotnames != Py_None)
+    /* clear all slots besides _p_*
+     * ( for backward-compatibility reason we do this only if class does not
+     *   override __new__ ) */
+    if (Py_TYPE(self)->tp_new == Pertype.tp_new)
     {
-        int i;
-
-        for (i = 0; i < PyList_GET_SIZE(slotnames); i++)
+        slotnames = pickle_slotnames(Py_TYPE(self));
+        if (slotnames && slotnames != Py_None)
         {
-            PyObject *name;
-            char *cname;
-            int is_special;
+            int i;
 
-            name = PyList_GET_ITEM(slotnames, i);
-#ifdef PY3K
-            if (PyUnicode_Check(name))
+            for (i = 0; i < PyList_GET_SIZE(slotnames); i++)
             {
-                PyObject *converted = convert_name(name);
-                cname = PyBytes_AS_STRING(converted);
+                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);
+                if (PyBytes_Check(name))
+                {
+                    cname = PyBytes_AS_STRING(name);
 #endif
-                is_special = !strncmp(cname, "_p_", 3);
+                    is_special = !strncmp(cname, "_p_", 3);
 #ifdef PY3K
-                Py_DECREF(converted);
+                    Py_DECREF(converted);
 #endif
-                if (is_special) /* skip persistent */
-                {
-                    continue;
+                    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();
+                /* 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);
     }
-    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

persistent/persistence.py

@@ -424,8 +424,11 @@ class Persistent(object):
         if idict is not None:
             idict.clear()
         type_ = type(self)
-        for slotname in Persistent._slotnames(self, _v_exclude=False):
-            getattr(type_, slotname).__delete__(self)
+        # ( for backward-compatibility reason we release __slots__ only if
+        #   class does not override __new__ )
+        if type_.__new__ is Persistent.__new__:
+            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)

persistent/tests/test_persistence.py

@@ -1340,6 +1340,36 @@ class _Persistent_Base(object):
         self.assertEqual(list(jar._loaded), [])
         self.assertEqual(list(jar._registered), [])
 
+    def test__p_invalidate_from_changed_w_slots_compat(self):
+        # check that (for backward-compatibility reason) slots are not released
+        # for classes where __new__ is overwritten. Attributes in __dict__
+        # should be always released.
+        class Derived(self._getTargetClass()):
+            __slots__ = ('myattr1', 'myattr2', '__dict__')
+            def __new__(cls):
+                obj = cls.__base__.__new__(cls)
+                obj.myattr1 = 'value1'
+                obj.myattr2 = 'value2'
+                obj.foo = 'foo1' # .foo & .bar are in __dict__
+                obj.bar = 'bar2'
+                return obj
+        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')
+        self.assertEqual(inst.__dict__, {'foo': 'foo1', 'bar': 'bar2'})
+        inst._p_invalidate()
+        self.assertEqual(inst._p_status, 'ghost')
+        self.assertEqual(list(jar._loaded), [])
+        self.assertEqual(Derived.myattr1.__get__(inst), 'value1')
+        self.assertEqual(Derived.myattr2.__get__(inst), 'value2')
+        self.assertEqual(inst.__dict__, {})
+        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