Stackless issue python#93: Unify and fix tasklet.kill(), tasklet.throw() and tasklet.raise_exception()

Unify the methods and update the documentation and changelog.txt.
See changelog.txt for details.
Add a test cases for tasklet.kill, and tasklet.throw.

https://bitbucket.org/stackless-dev/stackless/issues/93
(grafted from 1399a7b0c676c77fd6d491e74fe535f633529f0b and 0406cb0e9b05)

Author: Anselm Kruis

Doc/library/stackless/tasklets.rst

@@ -267,6 +267,14 @@ The ``tasklet`` class
    <slp-exc-section>`, and the purpose of the :ref:`TaskletExit <slp-exc>`
    exception.
 
+   If you try to raise an exception on a tasklet, that is not alive, the method
+   fails, except if *exc_class* is :exc:`TaskletExit` and the tasklet already ended.
+
+   .. versionchanged:: 3.3.7
+
+      In case of an error Stackless versions before 3.3.7 raise ``exc_class(*args)``.
+      Later versions raises :exc:`RuntimeError`.
+
 .. method:: tasklet.throw(exc=None, val=None, tb=None, pending=False)
 
    Raise an exception on the given tasklet.  The semantics are similar
@@ -277,14 +285,22 @@ The ``tasklet`` class
    runnable and the caller continues.  Otherwise, the target will be inserted
    before the current tasklet in the queue and switched to immediately.
 
+   If you try to raise an exception on a tasklet, that is not alive, the method
+   raises :exc:`RuntimeError` on the caller. There is one exception:
+   you can safely raise :exc:`TaskletExit`, if the tasklet already ended.
+
 .. method:: tasklet.kill(pending=False)
 
-   Raises the :ref:`TaskletExit <slp-exc>` exception on the tasklet.
-   *pending* has the same meaning as for :meth:`tasklet.throw`.
+   Terminates the tasklet and unblocks it, if the tasklet was blocked
+   on a channel. If the tasklet already ran to its end, the method does
+   nothing. If the tasklet has no thread, the method simply ends the
+   tasklet. Otherwise it raises the :ref:`TaskletExit <slp-exc>` exception
+   on the tasklet. *pending* has the same meaning as for :meth:`tasklet.throw`.
 
    This can be considered to be shorthand for::
 
-       >>> t.throw(TaskletExit, pending=pending)
+       >>> if t.alive:
+       >>>     t.throw(TaskletExit, pending=pending)
 
 .. method:: tasklet.set_atomic(flag)
 

Stackless/changelog.txt

@@ -10,6 +10,26 @@ What's New in Stackless 3.X.X?
 
 *Release date: 20XX-XX-XX*
 
+- https://bitbucket.org/stackless-dev/stackless/issues/93
+  Unify tasklet.kill(), tasklet.throw() and tasklet.raise_exception().
+  They now behave almost identically. This affects the error handling in some
+  corner cases:
+  + Some crash bugs have been fixed. Stackless Python now raises RuntimeError
+    instead of crashing.
+  + It is not possible to raise an exception in a dead (== not alive) tasklet.
+    In general, if you try it, the method call fails with an appropriate
+    RuntimeError. However there are exceptions:
+  + For Stackless versions before 3.3.7
+    tasklet.raise_exception(exc_class, *args) raises exc_class(*args) instead
+    of RuntimeError.
+  + If you call tasklet.kill() or tasklet.throw(TaskletExit) or
+    tasklet.raise_exception(TaskletExit) and if the tasklet already ran to its
+    end (tasklet.frame is None), the call succeeds.
+  + If you call tasklet.kill() on a tasklet, that didn't run to its end
+    (tasklet.frame is not None), but has no thread (tasklet.thread_id == -1),
+    Stackless drops the frame and proceeds as if the tasklet ran to its end.
+  Most users won't notice the difference.
+
 - https://bitbucket.org/stackless-dev/stackless/issues/89
 - https://bitbucket.org/stackless-dev/stackless/issues/81
   Fix (crash) bugs during thread and interpreter shutdown.
@@ -69,7 +89,7 @@ What's New in Stackless 3.3.6?
 
 - Windows installer: update openssl to version 1.0.1l.
 
-- Windows installer: update tcl and tk to version 8.5.15.0 
+- Windows installer: update tcl and tk to version 8.5.15.0
   and tix to version 8.4.3.5. Python 2.7.9 uses the same versions.
 
 - https://bitbucket.org/stackless-dev/stackless/issue/60
@@ -336,7 +356,7 @@ The very beginning of Stackless
 			It turns out to make sense:
 			Try to let the innermost interpreter resolve some recusrions.
 <<<<<<< local
-			Find an equivalent of olde Py_UnwindToken. Probably the 
+			Find an equivalent of olde Py_UnwindToken. Probably the
 =======
 			Find an equivalent of old Py_UnwindToken. Probably the
 >>>>>>> other

Stackless/module/taskletobject.c

@@ -1077,39 +1077,114 @@ PyTasklet_Throw_M(PyTaskletObject *self, int pending, PyObject *exc,
 }
 
 static PyObject *
-impl_tasklet_throw(PyTaskletObject *self, int pending, PyObject *exc, PyObject *val, PyObject *tb)
+#if PY_VERSION_HEX < 0x03030700
+#define SLP_IMPL_THROW_BOMB_WITH_BOE 1
+_impl_tasklet_throw_bomb(PyTaskletObject *self, int pending, PyObject *bomb, int bomb_on_error)
+#else
+#undef SLP_IMPL_THROW_BOMB_WITH_BOE
+_impl_tasklet_throw_bomb(PyTaskletObject *self, int pending, PyObject *bomb)
+#endif
 {
     STACKLESS_GETARG();
     PyThreadState *ts = PyThreadState_GET();
-    PyObject *ret, *bomb, *tmpval;
+    PyObject *ret, *tmpval;
     int fail;
 
-    if (ts->st.main == NULL)
-        return PyTasklet_Throw_M(self, pending, exc, val, tb);
-
-    bomb = slp_exc_to_bomb(exc, val, tb);
-    if (bomb == NULL)
-        return NULL;
-
+    assert(bomb != NULL);
+    assert(PyBomb_Check(bomb));
     /* raise it directly if target is ourselves.  delayed exception makes
      * no sense in this case
      */
-    if (ts->st.current == self)
+    if (ts->st.current == self) {
+        assert(self->cstate->tstate == ts);
         return slp_bomb_explode(bomb);
+    }
 
-    /* don't attempt to send to a dead tasklet.
-     * f.frame is null for the running tasklet and a dead tasklet
-     * A new tasklet has a CFrame
+    /* Handle new or dead tasklets.
      */
-    if (self->cstate->tstate == NULL || (self->f.frame == NULL && self != self->cstate->tstate->st.current)) {
-        /* however, allow tasklet exit errors for already dead tasklets */
-        if (PyObject_IsSubclass(((PyBombObject*)bomb)->curexc_type, PyExc_TaskletExit)) {
+    if (slp_get_frame(self) == NULL) {
+        /* The tasklet is not alive.
+         * There are a few special cases:
+         *  - The purpose of raising exception TaskletExit is to end the tasklet. Therefore
+         *    it is no error, if the tasklet already run to its end.
+         *  - Otherwise we have to raise a RuntimeError.
+         */
+        if (!PyObject_IsSubclass(((PyBombObject*)bomb)->curexc_type, PyExc_TaskletExit) ||
+            (self->cstate->tstate == NULL && self->f.frame != NULL)) {
+            /* Error: the exception is not TaskletExit or the tasklet did not run to its end. */
+#ifdef SLP_IMPL_THROW_BOMB_WITH_BOE
+            if (bomb_on_error)
+                return slp_bomb_explode(bomb);
+#endif
             Py_DECREF(bomb);
-            Py_RETURN_NONE;
+            if (self->cstate->tstate == NULL) {
+                RUNTIME_ERROR("tasklet has no thread", NULL);
+            }
+            RUNTIME_ERROR("You cannot throw to a dead tasklet", NULL);
+        }
+        /* A TaskletExit exception. The tasklet already ended (== can't be
+         * resurrected by bind_thread).
+         * Simply end the tasklet.
+         */
+        /* next two if()... just for test coverage mesurement */
+        if (self->cstate->tstate != NULL) {
+            assert(self->cstate->tstate != NULL);
+        }
+        if (self->f.frame == NULL) {
+            assert(self->f.frame == NULL);
+        }
+        Py_DECREF(bomb);
+
+        /* Now obey the post conditions of tasklet.throw:
+         * 1. the tasklet is not blocked
+         */
+        if (self->next && self->flags.blocked) {
+            /* we claim the channel's reference */
+            slp_channel_remove_slow(self, NULL, NULL, NULL);
+        } else {
+            Py_INCREF(self);
         }
+        /* Obey the post conditions of throw():
+         * 2. the tasklet is not scheduled. This is also a precondition,
+         *    because a dead tasklet must not be scheduled.
+         */
+
+#if 0   /* disabled until https://bitbucket.org/stackless-dev/stackless/issues/81 is resolved */
+        assert(self->next == NULL && self->prev == NULL);
+#endif
+
+        /* Due to bugs the above assertion my not hold.
+         * Try to work around.
+         */
+        if (self->next) {
+            if (self->cstate->tstate != NULL) {
+                /* The tasklet has a tstate an is scheduled.
+                 * we can use the regular remove.
+                 */
+                slp_current_remove_tasklet(self);
+            } else {
+                /* The tasklet has no tstate, is not blocked on a channel.
+                 * This happens, if a thread ended, but the tasklet was
+                 * survived killing.
+                 */
+                SLP_HEADCHAIN_REMOVE(self, prev, next);
+            }
+            Py_DECREF(self);
+        }
+        Py_DECREF(self);  /* the ref from the channel */
+        Py_RETURN_NONE;
+    }
+    assert(self->cstate->tstate != NULL);
+    /* don't modify a tasklet on an uninitialised or dead thread */
+    if (pending && self->cstate->tstate->st.main == NULL) {
+#ifdef SLP_IMPL_THROW_BOMB_WITH_BOE
+        if (bomb_on_error)
+            return slp_bomb_explode(bomb);
+#endif
         Py_DECREF(bomb);
-        RUNTIME_ERROR("You cannot throw to a dead tasklet", NULL);
+        RUNTIME_ERROR("Target thread isn't initialised", NULL);
     }
+
     TASKLET_CLAIMVAL(self, &tmpval);
     TASKLET_SETVAL_OWN(self, bomb);
     if (!pending) {
@@ -1141,6 +1216,30 @@ impl_tasklet_throw(PyTaskletObject *self, int pending, PyObject *exc, PyObject *
     return ret;
 }
 
+static PyObject *
+impl_tasklet_throw(PyTaskletObject *self, int pending, PyObject *exc, PyObject *val, PyObject *tb)
+{
+    STACKLESS_GETARG();
+    PyThreadState *ts = PyThreadState_GET();
+    PyObject *ret, *bomb;
+
+    if (ts->st.main == NULL)
+        return PyTasklet_Throw_M(self, pending, exc, val, tb);
+
+    bomb = slp_exc_to_bomb(exc, val, tb);
+    if (bomb == NULL)
+        return NULL;
+
+    STACKLESS_PROMOTE_ALL();
+#ifdef SLP_IMPL_THROW_BOMB_WITH_BOE
+    ret = _impl_tasklet_throw_bomb(self, pending, bomb, 0);
+#else
+    ret = _impl_tasklet_throw_bomb(self, pending, bomb);
+#endif
+    STACKLESS_ASSERT();
+    return ret;
+}
+
 int PyTasklet_Throw(PyTaskletObject *self, int pending, PyObject *exc,
                              PyObject *val, PyObject *tb)
 {
@@ -1191,27 +1290,22 @@ impl_tasklet_raise_exception(PyTaskletObject *self, PyObject *klass, PyObject *a
 {
     STACKLESS_GETARG();
     PyThreadState *ts = PyThreadState_GET();
-    PyObject *ret, *bomb, *tmpval;
-    int fail;
+    PyObject *ret, *bomb;
 
     if (ts->st.main == NULL)
         return PyTasklet_RaiseException_M(self, klass, args);
     bomb = slp_make_bomb(klass, args, "tasklet.raise_exception");
     if (bomb == NULL)
         return NULL;
-    if (ts->st.current == self)
-        return slp_bomb_explode(bomb);
-    /* if the tasklet is dead, do not run it (no frame) but explode */
-    if (slp_get_frame(self) == NULL)
-        return slp_bomb_explode(bomb);
 
-    TASKLET_CLAIMVAL(self, &tmpval);
-    TASKLET_SETVAL_OWN(self, bomb);
-    fail = slp_schedule_task(&ret, ts->st.current, self, stackless, 0);
-    if (fail)
-        TASKLET_SETVAL_OWN(self, tmpval);
-    else
-        Py_DECREF(tmpval);
+    STACKLESS_PROMOTE_ALL();
+#ifdef SLP_IMPL_THROW_BOMB_WITH_BOE
+    ret = _impl_tasklet_throw_bomb(self, 0, bomb, 1);
+#else
+    ret = _impl_tasklet_throw_bomb(self, 0, bomb);
+#endif
+    STACKLESS_ASSERT();
+
     return ret;
 }
 
@@ -1256,20 +1350,38 @@ impl_tasklet_kill(PyTaskletObject *task, int pending)
     STACKLESS_GETARG();
     PyObject *ret;
 
-    /*
-     * silently do nothing if the tasklet is dead.
-     * simple raising would kill ourself in this case.
+    /* We might be called without a thread state. If the tasklet
+     * still has a frame, impl_tasklet_throw() will raise
+     * RuntimeError. Therefore we need either to bind the tasklet to
+     * a thread or drop its frames. Both makes sense, but the documentation
+     * states, that Stackless does not silently change the thread of
+     * a tasklet. Therefore we drop the frames.
      */
-    if (slp_get_frame(task) == NULL) {
-        /* it can still be a new tasklet and not a dead one */
-        Py_CLEAR(task->f.cframe);
-        if (task->next) {
-            /* remove it from the run queue */
-            assert(!task->flags.blocked);
-            slp_current_remove_tasklet(task);
-            Py_DECREF(task);
+    assert(task->cstate);
+    if (task->cstate->tstate == NULL) {
+#ifdef SLP_TASKLET_KILL_REBINDS_THREAD
+        /* No thread state. Silently bind the tasklet to
+         * the current thread or drop its frames.
+         * Either action prevents an error in impl_tasklet_throw().
+         */
+        if (task->cstate->nesting_level == 0 && task->f.frame) {
+            /* rebind to the current thread */
+            PyObject *arg = PyTuple_New(0);
+            if (arg == NULL)
+                return NULL;
+            ret = tasklet_bind_thread((PyObject *)task, arg);
+            Py_DECREF(arg);
+            assert(ret != NULL); /* should not fail, if nesting_level is 0 */
+            if (ret == NULL)  /* in case of a bug */
+                return NULL;
+            Py_DECREF(ret);
+        } else {
+            Py_CLEAR(task->f.frame);  /* or better tasklet_clear_frames(task) ? */
         }
-        Py_RETURN_NONE;
+#else
+        /* drop the frame */
+        Py_CLEAR(task->f.frame);  /* or better tasklet_clear_frames(task) ? */
+#endif
     }
 
     /* we might be called after exceptions are gone */