3.14: Save/restore PyInterpreterFrame.stackpointer.

Author: jamadden

src/greenlet/TGreenlet.hpp

@@ -125,6 +125,7 @@ namespace greenlet
         //      Assertion `tstate->current_executor == NULL' failed.
         // see https://github.com/python-greenlet/greenlet/issues/460
         PyObject* current_executor;
+        _PyStackRef* stackpointer;
     #ifdef Py_GIL_DISABLED
         _PyCStackRef* c_stack_refs;
     #endif

src/greenlet/TPythonState.cpp

@@ -15,6 +15,7 @@ PythonState::PythonState()
 #if GREENLET_PY314
     ,py_recursion_depth(0)
     ,current_executor(nullptr)
+    ,stackpointer(nullptr)
     #ifdef Py_GIL_DISABLED
     ,c_stack_refs(nullptr)
     #endif
@@ -163,6 +164,14 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
     Py_XDECREF(frame);  // PyThreadState_GetFrame gives us a new
                         // reference.
     this->_top_frame.steal(frame);
+  #if GREENLET_PY314
+    if (this->top_frame()) {
+        this->stackpointer = this->_top_frame->f_frame->stackpointer;
+    }
+    else {
+        this->stackpointer = nullptr;
+    }
+  #endif
   #if GREENLET_PY313
     this->delete_later = Py_XNewRef(tstate->delete_later);
   #elif GREENLET_PY312
@@ -241,6 +250,11 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
     tstate->datastack_chunk = this->datastack_chunk;
     tstate->datastack_top = this->datastack_top;
     tstate->datastack_limit = this->datastack_limit;
+#if GREENLET_PY314 && defined(Py_GIL_DISABLED)
+    if (this->top_frame()) {
+        this->_top_frame->f_frame->stackpointer = this->stackpointer;
+    }
+#endif
     this->_top_frame.relinquish_ownership();
   #if GREENLET_PY313
     Py_XDECREF(tstate->delete_later);
@@ -295,6 +309,16 @@ int PythonState::tp_traverse(visitproc visit, void* arg, bool own_top_frame) noe
     if (own_top_frame) {
         Py_VISIT(this->_top_frame.borrow());
     }
+#if GREENLET_PY314
+    // TODO: Should we be visiting the c_stack_refs objects?
+    // CPython uses a specific macro to do that which takes into
+    // account boxing and null values and then calls
+    // ``_PyGC_VisitStackRef``, but we don't have access to that, and
+    // we can't duplicate it ourself (because it compares
+    // ``visitproc`` to another function we can't access).
+    // The naive way of looping over c_stack_refs->ref and visiting
+    // those crashes the process (at least with GIL disabled).
+#endif
     return 0;
 }
 

src/greenlet/TThreadState.hpp

@@ -66,18 +66,13 @@ namespace greenlet {
  * invoke destructors; the C++11 version is the most portable solution
  * I found). When the thread exits, we can drop references and
  * otherwise manipulate greenlets and frames that we know can no
- * longer be switched to. For compilers that don't support C++11
- * thread locals, we have a solution that uses the python thread
- * dictionary, though it may not collect everything as promptly as
- * other compilers do, if some other library is using the thread
- * dictionary and has a cycle or extra reference.
+ * longer be switched to.
  *
  * There are two small wrinkles. The first is that when the thread
  * exits, it is too late to actually invoke Python APIs: the Python
  * thread state is gone, and the GIL is released. To solve *this*
  * problem, our destructor uses ``Py_AddPendingCall`` to transfer the
- * destruction work to the main thread. (This is not an issue for the
- * dictionary solution.)
+ * destruction work to the main thread.
  *
  * The second is that once the thread exits, the thread local object
  * is invalid and we can't even access a pointer to it, so we can't

src/greenlet/TThreadStateCreator.hpp

@@ -15,6 +15,8 @@ namespace greenlet {
 
 typedef void (*ThreadStateDestructor)(ThreadState* const);
 
+// Only one of these, auto created per thread as a thread_local.
+// Constructing the state constructs the MainGreenlet.
 template<ThreadStateDestructor Destructor>
 class ThreadStateCreator
 {
@@ -36,8 +38,6 @@ class ThreadStateCreator
 
 public:
 
-    // Only one of these, auto created per thread.
-    // Constructing the state constructs the MainGreenlet.
     ThreadStateCreator() :
         _state((ThreadState*)1)
     {

src/greenlet/TThreadStateDestroy.cpp

@@ -33,6 +33,7 @@ struct ThreadState_DestroyNoGIL
     MarkGreenletDeadAndQueueCleanup(ThreadState* const state)
     {
 #if GREENLET_BROKEN_THREAD_LOCAL_CLEANUP_JUST_LEAK
+        // One rare platform.
         return;
 #endif
         // We are *NOT* holding the GIL. Our thread is in the middle
@@ -70,7 +71,11 @@ struct ThreadState_DestroyNoGIL
     static bool
     MarkGreenletDeadIfNeeded(ThreadState* const state)
     {
-        if (state && state->has_main_greenlet()) {
+        if (!state) {
+            return false;
+        }
+        LockGuard cleanup_lock(*mod_globs->thread_states_to_destroy_lock);
+        if (state->has_main_greenlet()) {
             // mark the thread as dead ASAP.
             // this is racy! If we try to throw or switch to a
             // greenlet from this thread from some other thread before
@@ -114,7 +119,7 @@ struct ThreadState_DestroyNoGIL
             // from the queue; its next iteration will go ahead and delete the item we just added.
             // And the pending call we schedule here will have no work to do.
             int result = AddPendingCall(
-                           PendingCallback_DestroyQueueWithGIL,
+                           PendingCallback_DestroyQueue,
                             nullptr);
             if (result < 0) {
                 // Hmm, what can we do here?
@@ -126,10 +131,11 @@ struct ThreadState_DestroyNoGIL
     }
 
     static int
-    PendingCallback_DestroyQueueWithGIL(void* UNUSED(arg))
+    PendingCallback_DestroyQueue(void* UNUSED(arg))
     {
-        // We're holding the GIL here, so no Python code should be able to
-        // run to call ``os.fork()``.
+        // We're may or may not be holding the GIL here (depending on
+        // Py_GIL_DISABLED), so calls to ``os.fork()`` may or may not
+        // be possible.
         while (1) {
             ThreadState* to_destroy;
             {
@@ -144,15 +150,15 @@ struct ThreadState_DestroyNoGIL
             // Drop the lock while we do the actual deletion.
             // This allows other calls to MarkGreenletDeadAndQueueCleanup
             // to enter and add to our queue.
-            DestroyOneWithGIL(to_destroy);
+            DestroyOne(to_destroy);
         }
         return 0;
     }
 
     static void
-    DestroyOneWithGIL(const ThreadState* const state)
+    DestroyOne(const ThreadState* const state)
     {
-        // Holding the GIL.
+        // May or may not be holding the GIL (depending on Py_GIL_DISABLED).
         // Passed a non-shared pointer to the actual thread state.
         // state -> main greenlet
         assert(state->has_main_greenlet());

src/greenlet/tests/test_gc.py

@@ -84,26 +84,3 @@ def greenlet_body():
         del g
         greenlet.getcurrent()
         gc.collect()
-
-    # def test_crashing_cycle(self):
-    #     # CPython 3.14 free threaded crashes on this
-    #     # (from docs/greenlet_gc.rst)
-    #     import doctest
-
-    #     def with_doctest():
-    #         """
-    #         >>> import gc
-    #         >>> from greenlet import getcurrent, greenlet, GreenletExit
-    #         >>> def collect_it():
-    #         ...      print("Collecting garbage")
-    #         ...      gc.collect()
-    #         >>> def outer():
-    #         ...     gc.collect()
-    #         >>> outer_glet = greenlet(outer)
-    #         >>> outer_glet.switch()
-    #         Collecting garbage
-    #         >>> print(list(globals()))
-    #         >>> print(list(locals()))
-    #         """
-
-    #     doctest.run_docstring_examples(with_doctest, dict())