Fix #323: Support Python 3.12

[mdboom]

Thanks for the review, @amotl.

Having updated it, and rerunning, it seems we are now running into a new set of test failures. This may be due to recent changes in 3.12, I guess. I will have to investigate further.

======================================================================
FAIL: test_unhandled_exception_in_greenlet_aborts (greenlet.tests.test_cpp.CPPTests.test_unhandled_exception_in_greenlet_aborts)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 316, in wrapper
    return _RefCountChecker(self, method)(args, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line [294](https://github.com/python-greenlet/greenlet/actions/runs/3480120383/jobs/5819513041#step:9:295), in __call__
    self._run_test(args, kwargs)
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 212, in _run_test
    self.function(self.testcase, *args, **kwargs)
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_cpp.py", line 67, in test_unhandled_exception_in_greenlet_aborts
    self._do_test_unhandled_exception(run_unhandled_exception_in_greenlet_aborts)
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_cpp.py", line 58, in _do_test_unhandled_exception
    self.assertEqual(p.exitcode, expected_exit)
AssertionError: -11 != -6

======================================================================
FAIL: test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main (greenlet.tests.test_leaks.TestLeaks.test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 445, in test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main
    self._check_untracked_memory_thread(deallocate_in_thread=False)
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 422, in _check_untracked_memory_thread
    self.assertEqual(EXIT_COUNT[0], 0)
AssertionError: 398 != 0

======================================================================
FAIL: test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread (greenlet.tests.test_leaks.TestLeaks.test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 439, in test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread
    self._check_untracked_memory_thread(deallocate_in_thread=True)
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 417, in _check_untracked_memory_thread
    self.assertEqual(EXIT_COUNT[0], ITER * thread_func.ITER)
AssertionError: 398 != 20000

----------------------------------------------------------------------

[mdboom]

I can't seem to reproduce these errors locally, with either CPython main or 3.12.0a1 (which seems to be what the CI here is using). I'm not sure how to debug this further.

[amotl]

Thank you, Michael. Apologies that I probably can't help on this matter. However, maybe those observations add some additional insights?

At [1], there is also:

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.12.0-alpha.1/x64/lib/python3.12/threading.py", line 1049, in _bootstrap_inner
  File "/opt/hostedtoolcache/Python/3.12.0-alpha.1/x64/lib/python3.12/threading.py", line 986, in run
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 392, in __call__
  File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 378, in run_it
RecursionError: maximum recursion depth exceeded

And at [2], there is:

terminate called after throwing an instance of 'exception_t'
Fatal Python error: Aborted

[1] https://github.com/python-greenlet/greenlet/actions/runs/3480120383/jobs/5819513041#step:9:237 [2] https://github.com/python-greenlet/greenlet/actions/runs/3480120383/jobs/5819513041#step:9:135

[tacaswell]

This has grown another compilation failure:

✔ 23:38:23 $ gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding/include/python3.12 -c src/greenlet/greenlet.cpp -o build/temp.linux-x86_64-cpython-312/src/greenlet/greenlet.o
In file included from src/greenlet/greenlet_internal.hpp:20,
                 from src/greenlet/greenlet.cpp:19:
src/greenlet/greenlet_greenlet.hpp: In member function ‘void greenlet::PythonState::operator<<(const PyThreadState*)’:
src/greenlet/greenlet_greenlet.hpp:847:42: error: ‘const PyThreadState’ {aka ‘const struct _ts’} has no member named ‘trash_delete_nesting’
  847 |     this->trash_delete_nesting = tstate->trash_delete_nesting;
      |                                          ^~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_greenlet.hpp: In member function ‘void greenlet::PythonState::operator>>(PyThreadState*)’:
src/greenlet/greenlet_greenlet.hpp:884:13: error: ‘PyThreadState’ {aka ‘struct _ts’} has no member named ‘trash_delete_nesting’
  884 |     tstate->trash_delete_nesting = this->trash_delete_nesting;
      |             ^~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet.cpp: In function ‘PyObject* mod_get_tstate_trash_delete_nesting(PyObject*)’:
src/greenlet/greenlet.cpp:3084:36: error: ‘PyThreadState’ {aka ‘struct _ts’} has no member named ‘trash_delete_nesting’
 3084 |     return PyLong_FromLong(tstate->trash_delete_nesting);
      |                                    ^~~~~~~~~~~~~~~~~~~~

[raphaelauv]

@mdboom thanks for the PR , you could try the CI with 3.12.0-alpha.5 ?

[hugovk]

3.12-dev points to the latest prerelease, which is currently 3.12.0-alpha.5 (with alpha 6 due next week: https://peps.python.org/pep-0693/), so how about keeping 3.12-dev and restarting the CI?

[tacaswell]

This is now failing with

$ gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -DCYTHON_FAST_THREAD_STATE=0 -fPIC -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding
/include/python3.12 -c src/greenlet/greenlet.cpp -o build/temp.linux-x86_64-cpython-312/src/greenlet/greenlet.o
In file included from src/greenlet/greenlet_internal.hpp:20,
                 from src/greenlet/greenlet.cpp:19:
src/greenlet/greenlet_greenlet.hpp: In member function ‘void greenlet::PythonState::operator<<(const PyThreadState*)’:
src/greenlet/greenlet_greenlet.hpp:847:42: error: ‘const PyThreadState’ {aka ‘const struct _ts’} has no member named ‘trash_delete_nesting’
  847 |     this->trash_delete_nesting = tstate->trash_delete_nesting;
      |                                          ^~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_greenlet.hpp: In member function ‘void greenlet::PythonState::operator>>(PyThreadState*)’:
src/greenlet/greenlet_greenlet.hpp:884:13: error: ‘PyThreadState’ {aka ‘struct _ts’} has no member named ‘trash_delete_nesting’
  884 |     tstate->trash_delete_nesting = this->trash_delete_nesting;
      |             ^~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet.cpp: In function ‘PyObject* mod_get_tstate_trash_delete_nesting(PyObject*)’:
src/greenlet/greenlet.cpp:3084:36: error: ‘PyThreadState’ {aka ‘struct _ts’} has no member named ‘trash_delete_nesting’
 3084 |     return PyLong_FromLong(tstate->trash_delete_nesting);
      |                                    ^~~~~~~~~~~~~~~~~~~~

which was removed in https://github.com/python/cpython/pull/101209

The following patch restores compilation:

diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp
index 5990323..cf3492e 100644
--- a/src/greenlet/greenlet.cpp
+++ b/src/greenlet/greenlet.cpp
@@ -3081,7 +3081,7 @@ static PyObject*
 mod_get_tstate_trash_delete_nesting(PyObject* UNUSED(module))
 {
     PyThreadState* tstate = PyThreadState_GET();
-    return PyLong_FromLong(tstate->trash_delete_nesting);
+    return PyLong_FromLong(tstate->trash.delete_nesting);
 }

 static PyMethodDef GreenMethods[] = {
diff --git a/src/greenlet/greenlet_greenlet.hpp b/src/greenlet/greenlet_greenlet.hpp
index 8e9228a..b08921c 100644
--- a/src/greenlet/greenlet_greenlet.hpp
+++ b/src/greenlet/greenlet_greenlet.hpp
@@ -844,7 +844,7 @@ void PythonState::operator<<(const PyThreadState *const tstate) G_NOEXCEPT
 #endif

     // All versions of Python.
-    this->trash_delete_nesting = tstate->trash_delete_nesting;
+    this->trash_delete_nesting = tstate->trash.delete_nesting;
 }

 void PythonState::operator>>(PyThreadState *const tstate) G_NOEXCEPT
@@ -881,7 +881,7 @@ void PythonState::operator>>(PyThreadState *const tstate) G_NOEXCEPT
     tstate->recursion_depth = this->recursion_depth;
 #endif
     // All versions of Python.
-    tstate->trash_delete_nesting = this->trash_delete_nesting;
+    tstate->trash.delete_nesting = this->trash_delete_nesting;
 }

 void PythonState::will_switch_from(PyThreadState *const origin_tstate) G_NOEXCEPT

but I have not tracked down if the upstream refactor is going to require a logical refactor on the greenlet side (as I do not actually understand what either side is doing!).

[tacaswell]

due to https://github.com/python/cpython/pull/103083 the full patch set I need to get greenlet to compile is:

diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp
index 5990323..cf3492e 100644
--- a/src/greenlet/greenlet.cpp
+++ b/src/greenlet/greenlet.cpp
@@ -3081,7 +3081,7 @@ static PyObject*
 mod_get_tstate_trash_delete_nesting(PyObject* UNUSED(module))
 {
     PyThreadState* tstate = PyThreadState_GET();
-    return PyLong_FromLong(tstate->trash_delete_nesting);
+    return PyLong_FromLong(tstate->trash.delete_nesting);
 }

 static PyMethodDef GreenMethods[] = {
diff --git a/src/greenlet/greenlet_greenlet.hpp b/src/greenlet/greenlet_greenlet.hpp
index 8e9228a..1213140 100644
--- a/src/greenlet/greenlet_greenlet.hpp
+++ b/src/greenlet/greenlet_greenlet.hpp
@@ -823,7 +823,6 @@ void PythonState::operator<<(const PyThreadState *const tstate) G_NOEXCEPT
       the switch, use `will_switch_from`.
     */
     this->cframe = tstate->cframe;
-    this->use_tracing = tstate->cframe->use_tracing;
 #endif
 #if GREENLET_PY311
     #if GREENLET_PY312
@@ -844,7 +843,7 @@ void PythonState::operator<<(const PyThreadState *const tstate) G_NOEXCEPT
 #endif

     // All versions of Python.
-    this->trash_delete_nesting = tstate->trash_delete_nesting;
+    this->trash_delete_nesting = tstate->trash.delete_nesting;
 }

 void PythonState::operator>>(PyThreadState *const tstate) G_NOEXCEPT
@@ -857,13 +856,6 @@ void PythonState::operator>>(PyThreadState *const tstate) G_NOEXCEPT
 #endif
 #if GREENLET_USE_CFRAME
     tstate->cframe = this->cframe;
-    /*
-      If we were tracing, we need to keep tracing.
-      There should never be the possibility of hitting the
-      root_cframe here. See note above about why we can't
-      just copy this from ``origin->cframe->use_tracing``.
-    */
-    tstate->cframe->use_tracing = this->use_tracing;
 #endif
 #if GREENLET_PY311
     #if GREENLET_PY312
@@ -881,18 +873,12 @@ void PythonState::operator>>(PyThreadState *const tstate) G_NOEXCEPT
     tstate->recursion_depth = this->recursion_depth;
 #endif
     // All versions of Python.
-    tstate->trash_delete_nesting = this->trash_delete_nesting;
+    tstate->trash.delete_nesting = this->trash_delete_nesting;
 }

 void PythonState::will_switch_from(PyThreadState *const origin_tstate) G_NOEXCEPT
 {
-#if GREENLET_USE_CFRAME
-    // The weird thing is, we don't actually save this for an
-    // effect on the current greenlet, it's saved for an
-    // effect on the target greenlet. That is, we want
-    // continuity of this setting across the greenlet switch.
-    this->use_tracing = origin_tstate->cframe->use_tracing;
-#endif
+
 }

 void PythonState::set_initial_state(const PyThreadState* const tstate) G_NOEXCEPT

There is probably more that can be pulled out of greenlet (as I think the use_tracking value is only used for restoring the the value to cframes).

Obviously this patch needs version gating....

[mdboom]

I have updated this to compile again (thanks @tacaswell for the pointers there).

There is now a different set of test failures that I'm working through (as well as a number of segfaults in the log):

Test failures with 3.12

``` ====================================================================== FAIL: test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main (greenlet.tests.test_leaks.TestLeaks.test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 445, in test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main self._check_untracked_memory_thread(deallocate_in_thread=False) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 422, in _check_untracked_memory_thread self.assertEqual(EXIT_COUNT[0], 0) AssertionError: 398 != 0 ====================================================================== FAIL: test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread (greenlet.tests.test_leaks.TestLeaks.test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 439, in test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread self._check_untracked_memory_thread(deallocate_in_thread=True) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_leaks.py", line 417, in _check_untracked_memory_thread self.assertEqual(EXIT_COUNT[0], ITER * thread_func.ITER) AssertionError: 398 != 20000 ====================================================================== FAIL: test_trace_events_from_greenlet_func_sets_profiler (greenlet.tests.test_tracing.TestPythonTracing.test_trace_events_from_greenlet_func_sets_profiler) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 316, in wrapper return _RefCountChecker(self, method)(args, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line [294](https://github.com/python-greenlet/greenlet/actions/runs/4820657398/jobs/8585461014?pr=327#step:9:295), in __call__ self._run_test(args, kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 212, in _run_test self.function(self.testcase, *args, **kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_tracing.py", line 181, in test_trace_events_from_greenlet_func_sets_profiler self._check_trace_events_from_greenlet_sets_profiler(greenlet.greenlet(run), File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_tracing.py", line 163, in _check_trace_events_from_greenlet_sets_profiler self.assertEqual(tracer.actions, [ AssertionError: Lists differ: [('re[87 chars]run')] != [('re[87 chars]run'), ('call', 'tpt_callback'), ('return', 't[55 chars]__')] Second list contains 4 additional elements. First extra element 4: ('call', 'tpt_callback') [('return', '__enter__'), ('call', 'tpt_callback'), ('return', 'tpt_callback'), - ('return', 'run')] ? ^ + ('return', 'run'), ? ^ + ('call', 'tpt_callback'), + ('return', 'tpt_callback'), + ('call', '__exit__'), + ('c_call', '__exit__')] ====================================================================== FAIL: test_trace_events_from_greenlet_subclass_sets_profiler (greenlet.tests.test_tracing.TestPythonTracing.test_trace_events_from_greenlet_subclass_sets_profiler) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line [316](https://github.com/python-greenlet/greenlet/actions/runs/4820657398/jobs/8585461014?pr=327#step:9:317), in wrapper return _RefCountChecker(self, method)(args, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 294, in __call__ self._run_test(args, kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 212, in _run_test self.function(self.testcase, *args, **kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_tracing.py", line 191, in test_trace_events_from_greenlet_subclass_sets_profiler self._check_trace_events_from_greenlet_sets_profiler(X(), tracer) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_tracing.py", line 163, in _check_trace_events_from_greenlet_sets_profiler self.assertEqual(tracer.actions, [ AssertionError: Lists differ: [('re[87 chars]run')] != [('re[87 chars]run'), ('call', 'tpt_callback'), ('return', 't[55 chars]__')] Second list contains 4 additional elements. First extra element 4: ('call', 'tpt_callback') [('return', '__enter__'), ('call', 'tpt_callback'), ('return', 'tpt_callback'), - ('return', 'run')] ? ^ + ('return', 'run'), ? ^ + ('call', 'tpt_callback'), + ('return', 'tpt_callback'), + ('call', '__exit__'), + ('c_call', '__exit__')] ====================================================================== FAIL: test_trace_events_multiple_greenlets_switching_siblings (greenlet.tests.test_tracing.TestPythonTracing.test_trace_events_multiple_greenlets_switching_siblings) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 316, in wrapper return _RefCountChecker(self, method)(args, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 294, in __call__ self._run_test(args, kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/leakcheck.py", line 212, in _run_test self.function(self.testcase, *args, **kwargs) File "/home/runner/work/greenlet/greenlet/src/greenlet/tests/test_tracing.py", line 269, in test_trace_events_multiple_greenlets_switching_siblings self.assertEqual(tracer.actions, [ AssertionError: Lists differ: [('re[90 chars]run')] != [('re[90 chars]run'), ('call', 'tpt_callback'), ('return', 't[55 chars]__')] Second list contains 4 additional elements. First extra element 4: ('call', 'tpt_callback') [('return', '__enter__'), ('call', 'tpt_callback'), ('return', 'tpt_callback'), - ('c_call', 'g1_run')] ? ^ + ('c_call', 'g1_run'), ? ^ + ('call', 'tpt_callback'), + ('return', 'tpt_callback'), + ('call', '__exit__'), + ('c_call', '__exit__')] ---------------------------------------------------------------------- ```

[mdboom]

The root cause of two of the test failures (test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main, test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread) appears to be a "maximum recursion depth exceeded" exception. I can make it pass by reducing the number of iterations from 10000 to 199 (200 fails). I don't understand the details of the 3.12 changes well enough, but either the recursion depth really just is smaller, or greenlet is trying to control the recursion limit and that technique no longer works.

@markshannon: I wonder if you have thoughts?

diff --git a/src/greenlet/tests/test_leaks.py b/src/greenlet/tests/test_leaks.py
index 0ed43b0..c0bfec5 100644
--- a/src/greenlet/tests/test_leaks.py
+++ b/src/greenlet/tests/test_leaks.py
@@ -366,7 +366,7 @@ class TestLeaks(TestCase):
                 raise
             return 1

-        ITER = 10000
+        ITER = 199
         def run_it():
             glets = []
             for _ in range(ITER):

[mdboom]

The other two failing tests (test_trace_events_multiple_greenlets_switching_siblings and test_trace_events_from_greenlet_subclass_sets_profiler) pass when using a Python with PEP669 merged in (i.e. a commit after this one). This makes sense, because prior to that, we would still need to be reading the use_tracing member, but after this we do not.

I don't know if it's worth the effort to get a newer 3.12 in CI now or just wait for the next alpha release to automatically fix these two failures.

[hugovk]

It's pretty easy to test 3.12 nightly with https://github.com/deadsnakes/action (let me know you'd like help setting it up).

But it's also only a week and a half until the next pre-release on 2023-05-08, which as it happens is the first beta (https://peps.python.org/pep-0693/#release-schedule).

[mdboom]

It's pretty easy to test 3.12 nightly with https://github.com/deadsnakes/action (let me know you'd like help setting it up).

An earlier revision of this PR did exactly that -- we can always go back to it. But longer term, 3.12-dev is probably the one we want anyway.

[mdboom]

The root cause of two of the test failures (test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_main, test_untracked_memory_doesnt_increase_unfinished_thread_dealloc_in_thread) appears to be a "maximum recursion depth exceeded" exception.

I have updated this PR to fix these failures by getting/restoring both the Python and C stack limits.

With that, the only remaining failures are related to pre-PEP669, and should be resolved when dev-3.12 updates to a version with PEP669.

[markshannon]

The change looks reasonable from a CPython perspective, but PEP 669 changes the way that tracing works internally and I'm not sure that greenlet's model matches. The failure of 3.12-dev, ubuntu-20.04 suggests something isn't quite right. Might we worth retesting now that #104387 has fixed some minor issues with tracing support.

[mdboom]

The failure of 3.12-dev, ubuntu-20.04 suggests something isn't quite right.

That was expected, since when this ran 3.12-dev didn't yet have PEP 669 merged. I have pushed a new commit to get it to re-run now, and it is passing.

PEP 669 changes the way that tracing works internally and I'm not sure that greenlet's model matches

Can you elaborate? Do you see this as fixable by porting to the PEP 669 API?

[jamadden]

Thank you everyone for all your work on this! It is greatly appreciated. Tests are passing with 3.12.0b2, so I'm going to get this merged so I can make a pre-release of greenlet ASAP.