Issue python#117: fix various reference leaks to frames.

Stackless used to leak references to frames / C-frames, if a tasklet didn't run to its end, or various other conditions.

This part of the fix changes the Stackless reference counting for frames to follow the general rules for Python objects. The concept of an "execute reference" no longer exists.
Frames no longer eat their reference. See Stackless/core/stackless_impl.h for a detailed explanation.

This patch adds a special frame reference counting debug mode, which can be enabled in stackless_tstate.h.

https://bitbucket.org/stackless-dev/stackless/issues/117
(grafted from 9e9a2cea45059e87d025a1867beb8b0e3369262d)

Author: Anselm Kruis

Objects/object.c

@@ -3,6 +3,9 @@
 
 #include "Python.h"
 #include "frameobject.h"
+#ifdef STACKLESS
+#include "core/stackless_impl.h"
+#endif
 
 #ifdef __cplusplus
 extern "C" {
@@ -1785,7 +1788,11 @@ _Py_Dealloc(PyObject *op)
 {
     destructor dealloc = Py_TYPE(op)->tp_dealloc;
     _Py_ForgetReference(op);
+#ifdef STACKLESS
+    SLP_WITH_VALID_CURRENT_FRAME((*dealloc)(op));
+#else
     (*dealloc)(op);
+#endif
 }
 
 /* Print all live objects.  Because PyObject_Print is called, the

Objects/typeobject.c

@@ -5742,8 +5742,7 @@ slp_tp_init_callback(PyFrameObject *f, int exc, PyObject *retval)
             cf->ob1 = NULL;
         }
     }
-    ts->frame = f;
-    Py_DECREF(cf);
+    SLP_STORE_NEXT_FRAME(ts, f);
     return STACKLESS_PACK(ts, retval);
 }
 #endif
@@ -5755,17 +5754,23 @@ slot_tp_init(PyObject *self, PyObject *args, PyObject *kwds)
     _Py_IDENTIFIER(__init__);
     PyObject *meth = lookup_method(self, &PyId___init__);
     PyObject *res;
+#ifdef STACKLESS
+    PyCFrameObject *f = NULL;
+#endif
 
     if (meth == NULL)
         return -1;
 #ifdef STACKLESS
     if (stackless) {
-        PyCFrameObject *f = slp_cframe_new(slp_tp_init_callback, 1);
+        f = slp_cframe_new(slp_tp_init_callback, 1);
         if (f == NULL)
             return -1;
         Py_INCREF(self);
         f->ob1 = self;
-        PyThreadState_GET()->frame = (PyFrameObject *) f;
+        SLP_SET_CURRENT_FRAME(PyThreadState_GET(), (PyFrameObject *) f);
+        /* f contains the only counted reference to current frame. This reference
+         * keeps the fame alive during the following PyObject_Call().
+         */
     }
 #endif
     STACKLESS_PROMOTE_ALL();
@@ -5774,10 +5779,16 @@ slot_tp_init(PyObject *self, PyObject *args, PyObject *kwds)
     Py_DECREF(meth);
 #ifdef STACKLESS
     if (stackless && !STACKLESS_UNWINDING(res)) {
-        /* required, because added a C-frame */
-        STACKLESS_PACK(PyThreadState_GET(), res);
+        /* required, because we added a C-frame */
+        PyThreadState *ts = PyThreadState_GET();
+        STACKLESS_PACK(ts, res);
+        assert(f);
+        assert((PyFrameObject *)f == SLP_CURRENT_FRAME(ts));
+        SLP_STORE_NEXT_FRAME(ts, (PyFrameObject *)f);
+        Py_DECREF(f);
         return STACKLESS_UNWINDING_MAGIC;
     }
+    Py_XDECREF(f);
     if (STACKLESS_UNWINDING(res)) {
         return STACKLESS_UNWINDING_MAGIC;
     }

Python/ceval.c

@@ -1167,8 +1167,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
     /* push frame */
     if (Py_EnterRecursiveCall("")) {
         Py_XDECREF(retval);
-        tstate->frame = f->f_back;
-        Py_DECREF(f);
+        SLP_STORE_NEXT_FRAME(tstate, f->f_back);
         return NULL;
     }
 #else
@@ -1220,7 +1219,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
 exit_eval_frame:
     Py_XDECREF(retval);
     Py_LeaveRecursiveCall();
-    tstate->frame = f->f_back;
+    SLP_STORE_NEXT_FRAME(tstate, f->f_back);
     return NULL;
 }
 
@@ -3738,8 +3737,7 @@ slp_eval_frame_value(PyFrameObject *f, int throwflag, PyObject *retval)
 
 #else
     Py_LeaveRecursiveCall();
-    tstate->frame = f->f_back;
-    Py_DECREF(f);
+    SLP_STORE_NEXT_FRAME(tstate, f->f_back);
     return retval;
 
 stackless_setup_with:
@@ -3766,16 +3764,23 @@ slp_eval_frame_value(PyFrameObject *f, int throwflag, PyObject *retval)
     /* the -1 is to adjust for the f_lasti change.
        (look for the word 'Promise' above) */
     f->f_lasti = INSTR_OFFSET() - 1;
-    if (tstate->frame->f_back != f)
+    if (SLP_PEEK_NEXT_FRAME(tstate)->f_back != f)
         return retval;
     STACKLESS_UNPACK(tstate, retval);
-    retval = tstate->frame->f_execute(tstate->frame, 0, retval);
-    if (tstate->frame != f) {
-        assert(f->f_execute == slp_eval_frame_value || f->f_execute == slp_eval_frame_noval ||
-            f->f_execute == slp_eval_frame_setup_with || f->f_execute == slp_eval_frame_with_cleanup);
-        if (f->f_execute == slp_eval_frame_noval)
-            f->f_execute = slp_eval_frame_value;
-        return retval;
+    {
+        PyFrameObject *f2 = SLP_CLAIM_NEXT_FRAME(tstate);
+        retval = CALL_FRAME_FUNCTION(f2, 0, retval);
+        Py_DECREF(f2);
+        if (SLP_PEEK_NEXT_FRAME(tstate) != f) {
+            assert(f->f_execute == slp_eval_frame_value || f->f_execute == slp_eval_frame_noval ||
+                f->f_execute == slp_eval_frame_setup_with || f->f_execute == slp_eval_frame_with_cleanup);
+            if (f->f_execute == slp_eval_frame_noval)
+                f->f_execute = slp_eval_frame_value;
+            return retval;
+        }
+        f2 = SLP_CLAIM_NEXT_FRAME(tstate);
+        assert(f == f2);
+        Py_DECREF(f2);
     }
     if (STACKLESS_UNWINDING(retval))
         STACKLESS_UNPACK(tstate, retval);
@@ -3800,14 +3805,14 @@ slp_eval_frame_value(PyFrameObject *f, int throwflag, PyObject *retval)
     goto stackless_call_return;
 
 stackless_interrupt_call:
+    /* interrupted during unwinding */
 
     f->f_execute = slp_eval_frame_noval;
     f->f_stacktop = stack_pointer;
 
     /* the -1 is to adjust for the f_lasti change.
        (look for the word 'Promise' above) */
     f->f_lasti = INSTR_OFFSET() - 1;
-    f = tstate->frame;
     return (PyObject *) Py_UnwindToken;
 #endif
 }
@@ -4172,17 +4177,23 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals,
     Py_INCREF(Py_None);
     retval = Py_None;
     if (stackless) {
-        tstate->frame = f;
+        SLP_STORE_NEXT_FRAME(tstate, f);
+        Py_DECREF(f);
         return STACKLESS_PACK(tstate, retval);
     }
     else {
-        if (f->f_back != NULL)
+        if (f->f_back != NULL) {
             /* use the faster path */
-            retval = slp_frame_dispatch(f, f->f_back, 0, retval);
-            else {
+            PyFrameObject *back = f->f_back;
+            Py_INCREF(back);
+            retval = slp_frame_dispatch(f, back, 0, retval);
+            Py_DECREF(back);
+        }
+        else {
             Py_DECREF(retval);
             retval = slp_eval_frame(f);
         }
+        Py_DECREF(f);
         return retval;
     }
 #else
@@ -4898,7 +4909,7 @@ fast_function(PyObject *func, PyObject ***pp_stack, int n, int na, int nk)
     PCALL(PCALL_FAST_FUNCTION);
     if (argdefs == NULL && co->co_argcount == n &&
         co->co_kwonlyargcount == 0 && nk==0 &&
-        co->co_flags == (CO_OPTIMIZED | CO_NEWLOCALS | CO_NOFREE)) {
+        (co->co_flags & (~PyCF_MASK)) == (CO_OPTIMIZED | CO_NEWLOCALS | CO_NOFREE)) {
         PyFrameObject *f;
         PyObject *retval = NULL;
         PyThreadState *tstate = PyThreadState_GET();
@@ -4928,10 +4939,21 @@ fast_function(PyObject *func, PyObject ***pp_stack, int n, int na, int nk)
         if (STACKLESS_POSSIBLE()) {
             Py_INCREF(Py_None);
             retval = Py_None;
-            tstate->frame = f;
+            SLP_STORE_NEXT_FRAME(tstate, f);
+            Py_DECREF(f);
             return STACKLESS_PACK(tstate, retval);
         }
-        return slp_eval_frame(f);
+        if (f->f_back != NULL) {
+            /* use the faster path */
+            PyFrameObject *back = f->f_back;
+            Py_INCREF(Py_None);
+            Py_INCREF(back);
+            retval = slp_frame_dispatch(f, back, 0, Py_None);
+            Py_DECREF(back);
+        }
+        else {
+            retval = slp_eval_frame(f);
+        }
 #else
         retval = PyEval_EvalFrameEx(f,0);
 #endif

Python/pystate.c

@@ -3,10 +3,9 @@
 
 #include "Python.h"
 #ifdef STACKLESS
-/* XXX this should vanish! */
-#include "compile.h"
 #include "frameobject.h"
 #endif
+#include "core/stackless_impl.h"
 
 /* --------------------------------------------------------------------------
 CAUTION
@@ -169,7 +168,7 @@ threadstate_getframe(PyThreadState *self)
 {
 #ifdef STACKLESS
     /* make sure to return a real frame */
-    struct _frame *f = self->frame;
+    struct _frame *f = SLP_CURRENT_FRAME(self);
     while (f != NULL && !PyFrame_Check(f))
         f = f->f_back;
     return f;
@@ -591,7 +590,7 @@ _PyThread_CurrentFrames(void)
         for (t = i->tstate_head; t != NULL; t = t->next) {
             PyObject *id;
             int stat;
-            struct _frame *frame = t->frame;
+            struct _frame *frame = SLP_PEEK_NEXT_FRAME(t);
             if (frame == NULL)
                 continue;
             id = PyLong_FromLong(t->thread_id);

Stackless/changelog.txt

@@ -30,8 +30,12 @@ What's New in Stackless 3.X.X?
   - Leak to an object, if a Python profile or trace function fails.
   - Various leaks, if stackless._wrap.frame.__setstate__() fails, because its
     state argument is invalid.
+  - Leak of references to Python stack frames or Stackless C-frames, if
+    a tasklet didn't run to its end or various other conditions. This part
+    of the fix changes the Stackless reference counting for frames to follow
+    the general rules for Python objects.
 
-  Additionally this change brings the handling of caught exceptions more in
+  Additionally, this change brings the handling of caught exceptions more in
   line with C-Python.
 
 - https://bitbucket.org/stackless-dev/stackless/issues/111

Stackless/core/cframeobject.c

@@ -66,10 +66,23 @@ cframe_traverse(PyCFrameObject *cf, visitproc visit, void *arg)
 static void
 cframe_clear(PyCFrameObject *cf)
 {
-    Py_CLEAR(cf->f_back);
-    Py_CLEAR(cf->ob1);
-    Py_CLEAR(cf->ob2);
-    Py_CLEAR(cf->ob3);
+    /* The Python C-API documentation recomends to use Py_CLEAR() to release
+     * references held by container objects. The following code is an unrolled
+     * version of four Py_CLEAR() macros. It is more robust at no additional
+     * costs.
+     */
+    PyFrameObject *tmp_f_back;
+    PyObject *tmp_ob1, *tmp_ob2, *tmp_ob3;
+    tmp_f_back = cf->f_back;
+    tmp_ob1 = cf->ob1;
+    tmp_ob2 = cf->ob2;
+    tmp_ob3 = cf->ob3;
+    cf->f_back = NULL;
+    cf->ob1 = cf->ob2 = cf->ob3 = NULL;
+    Py_XDECREF(tmp_f_back);
+    Py_XDECREF(tmp_ob1);
+    Py_XDECREF(tmp_ob2);
+    Py_XDECREF(tmp_ob3);
 }
 
 
@@ -93,8 +106,9 @@ slp_cframe_new(PyFrame_ExecFunc *exec, unsigned int linked)
         _Py_NewReference((PyObject *) cf);
     }
 
-    if (linked)
-        back = ts->frame;
+    if (linked) {
+        back = SLP_CURRENT_FRAME(ts);
+    }
     else
         back = NULL;
     Py_XINCREF(back);
@@ -203,7 +217,7 @@ static PyObject * run_cframe(PyFrameObject *f, int exc, PyObject *retval)
     PyTaskletObject *task = ts->st.current;
     int done = cf->i;
 
-    ts->frame = f;
+    SLP_SET_CURRENT_FRAME(ts, f);
 
     if (retval == NULL || done)
         goto exit_run_cframe;
@@ -218,18 +232,17 @@ static PyObject * run_cframe(PyFrameObject *f, int exc, PyObject *retval)
 
     if (STACKLESS_UNWINDING(retval)) {
         /* try to shortcut */
-        if (ts->st.current == task && ts->frame != NULL &&
-            ts->frame->f_back == (PyFrameObject *) cf) {
-            Py_CLEAR(ts->frame->f_back);
-            ts->frame->f_back = cf->f_back;
-            Py_DECREF(cf); /* the exec reference */
+        PyFrameObject *f;  /* a borrowed ref */
+        if (ts->st.current == task && (f = SLP_PEEK_NEXT_FRAME(ts)) != NULL &&
+            f->f_back == (PyFrameObject *) cf) {
+                Py_XINCREF(cf->f_back);
+                Py_SETREF(f->f_back, cf->f_back);
         }
         return retval;
     }
     /* pop frame */
 exit_run_cframe:
-    ts->frame = cf->f_back;
-    Py_DECREF(cf);
+    SLP_STORE_NEXT_FRAME(ts, cf->f_back);
     return retval;
 }
 

Stackless/core/slp_transfer.c

@@ -121,6 +121,7 @@ slp_transfer(PyCStackObject **cstprev, PyCStackObject *cst,
 
     /* since we change the stack we must assure that the protocol was met */
     STACKLESS_ASSERT();
+    SLP_ASSERT_FRAME_IN_TRANSFER(ts);
 
     if ((intptr_t *) &ts > ts->st.cstack_base)
         return climb_stack_and_transfer(cstprev, cst, prev);
@@ -149,6 +150,7 @@ slp_transfer(PyCStackObject **cstprev, PyCStackObject *cst,
     _cst = cst;
     _prev = prev;
     result = slp_switch();
+    SLP_ASSERT_FRAME_IN_TRANSFER(ts);
     if (!result) {
         if (_cst) {
             /* record the context of the target stack.  Can't do it before the switch because