Stackless issue python#239: fix issues found during review

- better documentation of the concept
- fix a left-over from the time, when setting the context of a main-
  tasklet was not supported
- fix the context handling, when the main tasklet starts/ends. Copy the
  context from/to the thread state.

Author: Anselm Kruis

Doc/library/stackless/tasklets.rst

@@ -609,27 +609,27 @@ Design requirements were
 4. Enable the integration of tasklet-based co-routines into the :mod:`asyncio` framework.
    This is an obvious application which involves context variables and tasklets.
 
-Now each tasklet object has a private context attribute, which is either undefined (``NULL``) or a
-:class:`~contextvars.Context` object. The design goals have some consequences:
+Now each tasklet object has it own private context attribute. The design goals have some consequences:
 
 * The active :class:`~contextvars.Context` object of a thread (as defined by the |PPL|)
   is the context of the :attr:`~stackless.current` tasklet. This implies that a tasklet switch,
   switches the active context of the thread.
 
 * In accordance with the design decisions made in :pep:`567` the context of a tasklet can't be
-  accessed directly, but you can use the method :meth:`tasklet.context_run` to run arbitrary code
+  accessed directly [#f1]_, but you can use the method :meth:`tasklet.context_run` to run arbitrary code
   in this context. For instance ``tasklet.context_run(contextvars.copy_context())`` returns a copy
   of the context.
   The attribute :attr:`tasklet.context_id` can be used to test, if two tasklets share the context.
 
-* A tasklet, whose context is undefined must behave identically to a tasklet, whose context is an
-  empty :class:`~contextvars.Context` object. [#f1]_ Therefore the |PY| API provides no way to distinguish
-  both states.
-
-* Whenever the context of a tasklet is to be shared with another tasklet and the context is initially
-  undefined, it must be set to a newly created :class:`~contextvars.Context` object beforehand.
+* If you use the C-API, the context attribute of a tasklet is stored in the field *context* of the structure
+  :c:type:`PyTaskletObject` or :c:type:`PyThreadState`. This field is is either undefined (``NULL``) or a pointer to a
+  :class:`~contextvars.Context` object.
+  A tasklet, whose *context* is ``NULL`` **must** behave identically to a tasklet, whose context is an
+  empty :class:`~contextvars.Context` object [#f2]_. Therefore the |PY| API provides no way to distinguish
+  both states. Whenever the context of a tasklet is to be shared with another tasklet and `tasklet->context`
+  is initially `NULL`, it must be set to a newly created :class:`~contextvars.Context` object beforehand.
   This affects the methods :meth:`~tasklet.context_run`, :meth:`~tasklet.__init__`, :meth:`~tasklet.bind`
-  and :meth:`~tasklet.__setstate__`.
+  and :meth:`~tasklet.__setstate__` as well as the attribute :attr:`tasklet.context_id`.
 
 * If the state of a tasklet changes from *not alive* to *bound* or to *alive* (methods :meth:`~tasklet.__init__`,
   :meth:`~tasklet.bind` or :meth:`~tasklet.__setstate__`), the context
@@ -645,7 +645,11 @@ Now each tasklet object has a private context attribute, which is either undefin
 
 .. rubric:: Footnotes
 
-.. [#f1]   Setting a context variable to a non default value sets a previously undefined
-           context attribute to a newly created :class:`~contextvars.Context` object. This can happen anytime in a
+.. [#f1]   Not exactly true. The return value of :meth:`tasklet.__reduce_ex__` can contain references to class
+           :class:`contextvars.Context`, but it is strongly discouraged, to use them for any other purpose
+           than pickling.
+
+.. [#f2]   Setting a context variable to a non default value changes the value of the field *context* from ``NULL``
+           to a pointer to a newly created :class:`~contextvars.Context` object. This can happen anytime in a
            library call. Therefore any difference between an undefined context and an empty context causes ill defined
            behavior.

Include/slp_structs.h

@@ -134,7 +134,10 @@ typedef struct _tasklet {
     int recursion_depth;
     PyObject *def_globals;
     PyObject *tsk_weakreflist;
-    PyObject *context;  /* if running: the saved context, otherwise the context for the tasklet */
+    /* If the tasklet is current: the context, the value of ts->context when the main tasklet was created.
+     *   (The context of a current tasklet is always ints->tasklet.)
+     * If the tasklet is not current: the context for the tasklet */
+    PyObject *context;
 } PyTaskletObject;
 
 

Stackless/module/scheduling.c

@@ -1298,6 +1298,9 @@ slp_initialize_main_and_current(void)
     assert(task->exc_state.exc_traceback == NULL);
     assert(task->exc_state.previous_item == NULL);
     assert(task->exc_info == &task->exc_state);
+    assert(task->context == NULL);
+    Py_XINCREF(ts->context);
+    task->context = ts->context;
     SLP_EXCHANGE_EXCINFO(ts, task);
 
     NOTIFY_SCHEDULE(ts, NULL, task, -1);
@@ -1393,6 +1396,9 @@ schedule_task_destruct(PyObject **retval, PyTaskletObject *prev, PyTaskletObject
         assert(ts->exc_info == &prev->exc_state);
         SLP_EXCHANGE_EXCINFO(ts, prev);
         TASKLET_CLAIMVAL(prev, retval);
+        Py_XINCREF(prev->context);
+        Py_XSETREF(ts->context, prev->context);
+        ts->context_ver++;
         if (PyBomb_Check(*retval))
             *retval = slp_bomb_explode(*retval);
     }

Stackless/module/taskletobject.c

@@ -434,10 +434,8 @@ PyTasklet_BindEx(PyTaskletObject *task, PyObject *func, PyObject *args, PyObject
 
     /*
      * Set the context to the current context. It can be changed later on.
-     * But only for non-main tasklest, because tasklet.set_context must not
-     * be used for a main tasklet.
      */
-    if (func && !(ts && task == ts->st.main))
+    if (func)
         if (_tasklet_init_context(task))
             return -1;
 

Stackless/unittests/test_miscell.py

@@ -1752,6 +1752,51 @@ def get_cid():
         self.assertNotEqual(cid0, cid)
         self.assertEqual(id(ctx), cid)
 
+    def test_main_tasklet_init(self):
+        # This test succeeds, if Stackless copies ts->context to into the main
+        # tasklet, when Stackless creates the main tasklet.
+        # This is important, if there is already a context set, when the interpreter
+        # gets called. Example: an interactive python prompt.
+        # See also: test_main_tasklet_fini
+        cid1 = stackless.current.context_id
+        cid2 = None
+        def task():
+            nonlocal cid2
+            cid2 = stackless.main.context_id
+
+        stackless.tasklet(task)()
+        stackless.test_outside()
+        self.assertEqual(stackless.current.context_id, cid1)
+        self.assertEqual(cid1, cid2)
+
+    def test_main_tasklet_fini(self):
+        # for a main tasklet of a thread initially ts->context == NULL
+        # This test succeeds, if Stackless copies the context of the main
+        # tasklet to ts->context after the main tasklet exits
+        # This way the last context of the main tasklet is preserved and available
+        # on the next invocation of the interpreter.
+        ctx_holder1 = None  # use a tasklet to keep the context alive
+        ctx_holder2 = None
+        def task():
+            nonlocal ctx_holder1
+            ctx_holder1 = stackless.main.context_run(stackless.tasklet, id)
+            self.assertEqual(ctx_holder1.context_id, stackless.main.context_id)
+
+        t = stackless.tasklet(task, ())
+        def other_thread():
+            nonlocal ctx_holder2
+            t.bind_thread()
+            t.insert()
+            stackless.test_outside()
+            ctx_holder2 = stackless.tasklet(id)
+            self.assertEqual(ctx_holder2.context_id, stackless.current.context_id)
+
+        tr = threading.Thread(target=other_thread, name="other thread")
+        tr.start()
+        tr.join()
+        self.assertIsNot(ctx_holder1, ctx_holder2)
+        self.assertEqual(ctx_holder1.context_id, ctx_holder2.context_id)
+
 
 #///////////////////////////////////////////////////////////////////////////////