Stackless issue python#92: GC fixes, cstate fixes and parameter tests

tasklet.bind() an tasklet.bind_thread() now correctly use the nesting level
to test for C-state. Additionally I renamed the static method
tasklet_has_c_stack() to tasklet_has_c_stack_and_thread().
The test in  tasklet_has_c_stack() was not correct, if the
tasklet was the current tasklet of its thread. (GC indirectly
calls this method for all tasklets from arbitrary threads.)

Additional related changes:
- This commit changes an assertion to allow deallocation of tasklets with
a non zero C-stack but no tstate.
- The commit adds a warning message about deallocation tasklets with non
zero C-stack, if Py_VerboseFlag is non zero.
- On deallocation of a tasklet, zero the "task" pointer of its cstate.

The commit adds a test case for tasklet.bind() and tasklet.bind_thread()
on a tasklet, that has C-state but no thread-state.

https://bitbucket.org/stackless-dev/stackless/issues/92
(grafted from 7c71dffe45072a070aa59d9c97b593d89d0be18e, b79ce9e731a5,
2a10937f0ab9, a5c487b3bd51, b171a8b3ee7d and b45e6e4786f9)

Author: Anselm Kruis

Doc/library/stackless/threads.rst

@@ -38,8 +38,11 @@ will report as ``-1``. This also includes soft-switched tasklets,
 which share a C-state.
 
 The reason Stackless kills tasklets with C-state is that not doing so
-can cause serious leaks when a C-state is not unwound. Stackless cannot 
-kill soft-switched tasklets, because there is no central list of them. 
+can cause serious leaks when a C-state is not unwound. If Stackless runs
+in verbose mode (command line option :option:`-v` or :envvar:`PYTHONVERBOSE`),
+Stackless prints a warning message, if it deallocates a tasklet
+with a C-state. Stackless cannot
+kill soft-switched tasklets, because there is no central list of them.
 Stackless only knows about the hard-switched ones.
 
 Threads that end really should make sure that they finish whatever worker 

Python/pystate.c

@@ -342,7 +342,7 @@ PyThreadState_Clear(PyThreadState *tstate)
 #endif
     if (Py_VerboseFlag && tstate->frame != NULL)
         fprintf(stderr,
-          "PyThreadState_Clear: warning: thread still has a frame\n");
+        "# PyThreadState_Clear: warning: thread still has a frame\n");
 
     Py_CLEAR(tstate->frame);
 

Stackless/changelog.txt

@@ -10,6 +10,13 @@ What's New in Stackless 3.X.X?
 
 *Release date: 20XX-XX-XX*
 
+- https://bitbucket.org/stackless-dev/stackless/issues/92  
+  If you run Stackless with the option '-v' (or set the environment variable
+  PYTHONVERBOSE), Stackless prints a warning message, if it deallocates a
+  tasklet, that has C-state.
+  Additionally, the methods tasklet.bind() and tasklet.bind_thread() now check
+  correctly, if the tasklet has C-state.
+
 - https://bitbucket.org/stackless-dev/stackless/issues/91
   Stackless now resets the recursion depth, if you re-bind
   a tasklet to another callable.

Stackless/module/taskletobject.c

@@ -94,9 +94,12 @@ slp_current_unremove(PyTaskletObject* task)
  * problems elsewhere (why didn't the tasklet die when instructed to?)
  */
 static int
-tasklet_has_c_stack(PyTaskletObject *t)
+tasklet_has_c_stack_and_thread(PyTaskletObject *t)
 {
-    return t->f.frame && t->cstate && t->cstate->tstate && t->cstate->nesting_level != 0 ;
+    /* The GC may call this function for a current tasklet.
+     * Therefore we need the complete check. */
+    return t->f.frame && t->cstate && t->cstate->tstate &&
+        (t->cstate->tstate->st.current == t ? t->cstate->tstate->st.nesting_level : t->cstate->nesting_level) != 0;
 }
 
 static int
@@ -107,7 +110,7 @@ tasklet_traverse(PyTaskletObject *t, visitproc visit, void *arg)
     /* tasklets that need to be switched to for the kill, can't be collected.
      * Only trivial decrefs are allowed during GC collect
      */
-    if (tasklet_has_c_stack(t))
+    if (tasklet_has_c_stack_and_thread(t))
         PyObject_GC_Collectable((PyObject *)t, visit, arg, 0);
 
     /* we own the "execute reference" of all the frames */
@@ -186,7 +189,7 @@ static void
 tasklet_dealloc(PyTaskletObject *t)
 {
     PyObject_GC_UnTrack(t);
-    if (tasklet_has_c_stack(t)) {
+    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,
@@ -205,7 +208,13 @@ tasklet_dealloc(PyTaskletObject *t)
     if (t->tsk_weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *)t);
     if (t->cstate != NULL) {
-        assert(t->cstate->task != t || Py_SIZE(t->cstate) == 0);
+        assert(t->cstate->task != t || Py_SIZE(t->cstate) == 0 || t->cstate->tstate == NULL);
+        if (t->cstate->task == t) {
+            t->cstate->task = NULL;
+            if (Py_VerboseFlag && Py_SIZE(t->cstate) != 0) {
+                PySys_WriteStderr("# tasklet_dealloc: warning: tasklet %p has a non zero C-stack. \n", (void*)t);
+            }
+        }
         Py_DECREF(t->cstate);
     }
     Py_DECREF(t->tempval);
@@ -248,7 +257,7 @@ PyTasklet_BindEx(PyTaskletObject *task, PyObject *func, PyObject *args, PyObject
     if (PyTasklet_Scheduled(task)) {
         RUNTIME_ERROR("tasklet is scheduled", -1);
     }
-    if (tasklet_has_c_stack(task)) {
+    if (PyTasklet_GetNestingLevel(task)) {
         RUNTIME_ERROR("tasklet has C state on its stack", -1);
     }
     if (ts && task == ts->st.main && args == NULL && kwargs == NULL) {
@@ -569,7 +578,7 @@ tasklet_bind_thread(PyObject *self, PyObject *args)
     if (PyTasklet_Scheduled(task) && !task->flags.blocked) {
         RUNTIME_ERROR("can't (re)bind a runnable tasklet", NULL);
     }
-    if (tasklet_has_c_stack(task)) {
+    if (PyTasklet_GetNestingLevel(task)) {
         RUNTIME_ERROR("tasklet has C state on its stack", NULL);
     }
     if (target_tid != -1) {

Stackless/unittests/test_miscell.py

@@ -9,6 +9,8 @@
 import weakref
 import types
 import _thread as thread
+import time
+from stackless import _test_nostacklesscall as apply
 
 from support import StacklessTestCase, AsTaskletTestCase
 try:
@@ -952,6 +954,34 @@ def test():
         self.assertEqual(tlet.recursion_depth, 0)
         self.assertEqual(self.recursion_depth_in_test, 1)
 
+    def test_unbind_fail_cstate_no_thread(self):
+        # https://bitbucket.org/stackless-dev/stackless/issues/92
+        loop = True
+
+        def task():
+            while loop:
+                try:
+                    stackless.main.switch()
+                except TaskletExit:
+                    pass
+
+        def other_thread_main():
+            tlet.bind_thread()
+            tlet.run()
+
+        tlet = stackless.tasklet().bind(apply, (task,))
+        t = threading.Thread(target=other_thread_main, name="other thread")
+        t.start()
+        t.join()
+        time.sleep(0.05)  # other_thread needs some time to be destroyed
+
+        loop = False
+        self.assertEqual(tlet.thread_id, -1)
+        self.assertFalse(tlet.alive)
+        self.assertFalse(tlet.restorable)
+        self.assertGreater(tlet.nesting_level, 0)
+        self.assertRaisesRegex(RuntimeError, "tasklet has C state on its stack", tlet.bind, None)
+
 
 class TestSwitch(StacklessTestCase):
     """Test the new tasklet.switch() method, which allows

Stackless/unittests/test_thread.py

@@ -4,6 +4,7 @@
 import stackless
 import sys
 import time
+from stackless import _test_nostacklesscall as apply
 
 from support import StacklessTestCase, AsTaskletTestCase
 try:
@@ -391,6 +392,40 @@ def test_rebind_from_dead(self):
         t.bind_thread()
         self.assertEqual(t.thread_id, stackless.getcurrent().thread_id)
 
+    def test_rebind_from_dead_fail_cstate(self):
+        # A test for https://bitbucket.org/stackless-dev/stackless/issues/92
+        loop = True
+
+        def task():
+            while loop:
+                try:
+                    stackless.main.switch()
+                except TaskletExit:
+                    pass
+
+        def other_thread_main():
+            tlet.bind_thread()
+            tlet.run()
+
+        tlet = stackless.tasklet().bind(apply, (task,))
+        t = threading.Thread(target=other_thread_main, name="other thread")
+        t.start()
+        t.join()
+        time.sleep(0.1)  # other_thread needs some time to be destroyed
+
+        self.assertEqual(tlet.thread_id, -1)
+        self.assertFalse(tlet.alive)
+        self.assertFalse(tlet.restorable)
+        self.assertGreater(tlet.nesting_level, 0)
+        loop = False
+        try:
+            self.assertRaisesRegex(RuntimeError, "tasklet has C state on its stack", tlet.bind_thread)
+        except AssertionError:
+            tlet.kill()  # causes an assertion error in debug builds of 2.7.9-slp
+            raise
+        # the tasklet has no thread
+        self.assertEqual(tlet.thread_id, -1)
+
     def test_methods_on_dead(self):
         """test that tasklet methods on a dead tasklet behave well"""
         class MyException(Exception):