Stackless issue python#192: use tp_finalize for complex finalisation code

Refactor the finalisation code of stackless.tasklet and
stackless.channel.
- For tasklets, use a PEP 442 finalizer;
- for channels a simple tp_clear function is sufficient.

Like every PEP 442 other finalizer, the new finalizer runs only once for
each tasklet.

Author: Anselm Kruis

Stackless/changelog.txt

@@ -9,6 +9,12 @@ What's New in Stackless 3.X.X?
 
 *Release date: 20XX-XX-XX*
 
+- https://github.com/stackless-dev/stackless/issues/192
+  Use the PEP 442 mechanic to finalize a tasklet instead of a arcane logic in
+  tp_dealloc. The new code calls the finalizer only once. The finalizer tries
+  to kill the tasklet, as long as it has an unfinished C-state. This change
+  brings the behavior more in line with generators.
+
 - https://github.com/stackless-dev/stackless/issues/193
   Fix the macros PyTasklet_CheckExact(op) and PyChannel_CheckExact(op).
   Previously they contained a reference to a non existing variable.

Stackless/core/stackless_impl.h

@@ -737,8 +737,6 @@ int slp_return_wrapper_hard(PyObject *retval);
 int slp_int_wrapper(PyObject *retval);
 int slp_current_wrapper(int(*func)(PyTaskletObject*),
                         PyTaskletObject *task);
-int slp_resurrect_and_kill(PyObject *self,
-                           void(*killer)(PyObject *));
 
 /* stackless pickling support */
 int slp_async_gen_init_hooks(PyAsyncGenObject *o);

Stackless/core/stackless_util.c

@@ -122,67 +122,6 @@ slp_current_wrapper( int(*func)(PyTaskletObject*), PyTaskletObject *task )
     return ret;
 }
 
-int
-slp_resurrect_and_kill(PyObject *self, void(*killer)(PyObject *))
-{
-    /* modelled after typeobject.c's slot_tp_del */
-
-    PyObject *error_type, *error_value, *error_traceback;
-
-    /* this is different from typeobject.c's slot_tp_del: our callers already 
-       called PyObject_GC_UnTrack(self) */
-    assert(PyObject_IS_GC(self) && ! _PyObject_GC_IS_TRACKED(self));
-
-    /* Temporarily resurrect the object. */
-    assert(Py_REFCNT(self) == 0);
-    Py_REFCNT(self) = 1;
-
-    /* Save the current exception, if any. */
-    PyErr_Fetch(&error_type, &error_value, &error_traceback);
-
-    killer(self);
-
-    /* Restore the saved exception. */
-    PyErr_Restore(error_type, error_value, error_traceback);
-
-    /* Undo the temporary resurrection; can't use DECREF here, it would
-     * cause a recursive call.
-     */
-    assert(Py_REFCNT(self) > 0);
-    if (--Py_REFCNT(self) == 0)
-        return 0;    /* this is the normal path out */
-
-    /* __del__ resurrected it!  Make it look like the original Py_DECREF
-     * never happened.
-     */
-    {
-        Py_ssize_t refcnt = Py_REFCNT(self);
-        _Py_NewReference(self);
-        Py_REFCNT(self) = refcnt;
-    }
-    /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so
-     * we need to undo that. */
-    _Py_DEC_REFTOTAL;
-    /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object
-     * chain, so no more to do there.
-     * If COUNT_ALLOCS, the original decref bumped tp_frees, and
-     * _Py_NewReference bumped tp_allocs:  both of those need to be
-     * undone.
-     */
-#ifdef COUNT_ALLOCS
-    --Py_TYPE(self)->tp_frees;
-    --Py_TYPE(self)->tp_allocs;
-#endif
-    /* This code copied from iobase_dealloc() (in 3.0) */
-    /* When called from a heap type's dealloc, the type will be
-       decref'ed on return (see e.g. subtype_dealloc in typeobject.c).
-       This will undo that, thus preventing a crash.  But such a type
-       _will_ have had its dict and slots cleared. */
-    if (PyType_HasFeature(Py_TYPE(self), Py_TPFLAGS_HEAPTYPE))
-        Py_INCREF(Py_TYPE(self));
-
-    return -1;
-}
 
 /*
  * A thread id is either an unsigned long or the special value -1.

Stackless/module/channelobject.c

@@ -11,9 +11,6 @@
 #include "core/stackless_impl.h"
 #include "channelobject.h"
 
-static void
-channel_remove_all(PyObject *ob);
-
 Py_LOCAL_INLINE(PyChannelFlagStruc)
 channel_flags_from_integer(int flags) {
 #if defined(SLP_USE_NATIVE_BITFIELD_LAYOUT) && SLP_USE_NATIVE_BITFIELD_LAYOUT
@@ -62,23 +59,19 @@ channel_traverse(PyChannelObject *ch, visitproc visit, void *arg)
 
 static void
 channel_clear(PyObject *ob)
-{
-    /* this function does nothing but decref, so it's safe to use */
-    channel_remove_all(ob);
-}
-
-static void
-channel_remove_all(PyObject *ob)
 {
     PyChannelObject *ch = (PyChannelObject *) ob;
+    assert(PyChannel_Check(ob));
 
     /*
      * remove all tasklets and hope they will die.
      * Note that the channel might receive new actions
      * during tasklet deallocation, so we don't even know
      * if it will have the same direction. :-/
      */
+    ch->flags.closing = 1;
     while (ch->balance) {
+        /* this function does nothing but setting task->flags.blocked = 0, so it's safe to use */
         ob = (PyObject *) slp_channel_remove(ch, NULL, NULL, NULL);
         Py_DECREF(ob);
     }
@@ -88,15 +81,10 @@ static void
 channel_dealloc(PyObject *ob)
 {
     PyChannelObject *ch = (PyChannelObject *) ob;
-    PyObject_GC_UnTrack(ob);
 
-    if (ch->balance) {
-        if (slp_resurrect_and_kill(ob, channel_remove_all)) {
-            /* the beast has grown new references */
-            PyObject_GC_Track(ob);
-            return;
-        }
-    }
+
+    PyObject_GC_UnTrack(ob);
+    channel_clear(ob);
     if (ch->chan_weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)ch);
     Py_TYPE(ob)->tp_free(ob);
@@ -1152,7 +1140,7 @@ channel_setstate(PyObject *self, PyObject *args)
                           &PyList_Type, &lis))
         return NULL;
 
-    channel_remove_all((PyObject *) ch);
+    channel_clear((PyObject *) ch);
     n = PyList_GET_SIZE(lis);
     ch->flags = channel_flags_from_integer(flags);
     dir = balance > 0 ? 1 : -1;

Stackless/module/taskletobject.c

@@ -249,24 +249,43 @@ exc_state_clear(_PyErr_StackItem *exc_state)
 }
 
 static void
-tasklet_dealloc(PyTaskletObject *t)
+tasklet_finalize(PyObject *self)
 {
-    PyObject_GC_UnTrack(t);
+    PyTaskletObject *t;
+    PyObject *error_type, *error_value, *error_traceback;
+
+    assert(PyTasklet_Check(self));
+    t = (PyTaskletObject *)self;
+
+    /* Save the current exception, if any. */
+    PyErr_Fetch(&error_type, &error_value, &error_traceback);
+
     if (tasklet_has_c_stack_and_thread(t)) {
         /*
          * we want to cleanly kill the tasklet in the case it
-         * was forgotten. One way would be to resurrect it,
-         * but this is quite ugly with many ifdefs, see
-         * classobject/typeobject.
-         * Well, we do it.
+         * was forgotten.
+         */
+        kill_finally(self);
+    }
+
+    /* Restore the saved exception. */
+    PyErr_Restore(error_type, error_value, error_traceback);
+}
+
+static void
+tasklet_dealloc(PyTaskletObject *t)
+{
+    if (PyTasklet_CheckExact(t)) {
+        /* When ob is subclass of stackless.tasklet, finalizer is called from
+         * subtype_dealloc.
          */
-        if (slp_resurrect_and_kill((PyObject *) t, kill_finally)) {
-            /* the beast has grown new references */
-            PyObject_GC_Track(t);
+        if (PyObject_CallFinalizerFromDealloc((PyObject *)t) < 0) {
+            // resurrected.
             return;
         }
     }
 
+    PyObject_GC_UnTrack(t);
     tasklet_clear_frames(t);
     if (t->tsk_weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)t);
@@ -2339,7 +2358,8 @@ PyTypeObject PyTasklet_Type = {
     PyObject_GenericSetAttr,            /* tp_setattro */
     0,                                  /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
-        Py_TPFLAGS_BASETYPE,            /* tp_flags */
+        Py_TPFLAGS_BASETYPE |
+        Py_TPFLAGS_HAVE_FINALIZE,       /* tp_flags */
     tasklet__doc__,                     /* tp_doc */
     (traverseproc)tasklet_traverse,     /* tp_traverse */
     (inquiry) tasklet_clear,            /* tp_clear */
@@ -2359,5 +2379,14 @@ PyTypeObject PyTasklet_Type = {
     0,                                  /* tp_alloc */
     tasklet_new,                        /* tp_new */
     PyObject_GC_Del,                    /* tp_free */
+    0,                                  /* tp_is_gc */
+    0,                                  /* tp_bases */
+    0,                                  /* tp_mro */
+    0,                                  /* tp_cache */
+    0,                                  /* tp_subclasses */
+    0,                                  /* tp_weaklist */
+    0,                                  /* tp_del */
+    0,                                  /* tp_version_tag */
+    tasklet_finalize,                   /* tp_finalize */
 };
 #endif