-
Notifications
You must be signed in to change notification settings - Fork 247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #323: Support Python 3.12 #327
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding support for Python 3.12. You may want to exercise my suggestions on how to improve CI/GHA setup, if you like them.
Also, it may make sense to add a corresponding line to https://github.com/python-greenlet/greenlet/blob/master/setup.py#L216, to indicate the package is ready for Python 3.12 already.
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.
|
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. |
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:
[1] https://github.com/python-greenlet/greenlet/actions/runs/3480120383/jobs/5819513041#step:9:237 |
This has grown another compilation failure:
|
@mdboom thanks for the PR , you could try the CI with |
|
This is now failing with
which was removed in python/cpython#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!). |
due to python/cpython#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 Obviously this patch needs version gating.... |
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
|
The root cause of two of the test failures ( @markshannon: I wonder if you have thoughts?
|
The other two failing tests ( 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. |
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). |
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. |
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 |
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. |
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.
Can you elaborate? Do you see this as fixable by porting to the PEP 669 API? |
#if GREENLET_PY311 | ||
#if GREENLET_PY312 | ||
this->py_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining; | ||
this->c_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be C_RECURSION_LIMIT - tstate->c_recursion_remaining
?
Co-authored-by: Andreas Motl <[email protected]>
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. |
Switch to wheel.mk. 3.0.1 (2023-10-25) ================== - Fix a potential crash on Python 3.8 at interpreter shutdown time. This was a regression from earlier 3.0.x releases. Reported by Matt Wozniski in `issue 376 <https://github.com/python-greenlet/greenlet/issues/376>`_. 3.0.0 (2023-10-02) ================== - No changes from 3.0rc3 aside from the version number. 3.0.0rc3 (2023-09-12) ===================== - Fix an intermittent error during process termination on some platforms (GCC/Linux/libstdc++). 3.0.0rc2 (2023-09-09) ===================== - Fix some potential bugs (assertion failures and memory leaks) in previously-untested error handling code. In some cases, this means that the process will execute a controlled ``abort()`` after severe trouble when previously the process might have continued for some time with a corrupt state. It is unlikely those errors occurred in practice. - Fix some assertion errors and potential bugs with re-entrant switches. - Fix a potential crash when certain compilers compile greenlet with high levels of optimization. The symptom would be that switching to a greenlet for the first time immediately crashes. - Fix a potential crash when the callable object passed to the greenlet constructor (or set as the ``greenlet.run`` attribute) has a destructor attached to it that switches. Typically, triggering this issue would require an unlikely subclass of ``greenlet.greenlet``. - Python 3.11+: Fix rare switching errors that could occur when a garbage collection was triggered during the middle of a switch, and Python-level code in ``__del__`` or weakref callbacks switched to a different greenlet and ultimately switched back to the original greenlet. This often manifested as a ``SystemError``: "switch returned NULL without an exception set." For context on the fixes, see `gevent issue #1985 <https://github.com/gevent/gevent/issues/1985>`_. 3.0.0rc1 (2023-09-01) ===================== - Windows wheels are linked statically to the C runtime in an effort to prevent import errors on systems without the correct C runtime installed. It's not clear if this will make the situation better or worse, so please share your experiences in `issue 346 <https://github.com/python-greenlet/greenlet/issues/346>`_. Note that this only applies to the binary wheels found on PyPI. Building greenlet from source defaults to the shared library. Set the environment variable ``GREENLET_STATIC_RUNTIME=1`` at build time to change that. - Build binary wheels for Python 3.12 on macOS. - Fix compiling greenlet on a debug build of CPython 3.12. There is `one known issue <https://github.com/python-greenlet/greenlet/issues/368>`_ that leads to an interpreter crash on debug builds. - Python 3.12: Fix walking the frame stack of suspended greenlets. Previously accessing ``glet.gr_frame.f_back`` would crash due to `changes in CPython's undocumented internal frame handling <https://github.com/python/cpython/commit/1e197e63e21f77b102ff2601a549dda4b6439455>`_. Platforms --------- - Now, greenlet *may* compile and work on Windows ARM64 using llvm-mingw, but this is untested and unsupported. See `PR <https://github.com/python-greenlet/greenlet/pull/224>`_ by Adrian Vladu. - Now, greenlet *may* compile and work on LoongArch64 Linux systems, but this is untested and unsupported. See `PR 257 <https://github.com/python-greenlet/greenlet/pull/257/files>`_ by merore. Known Issues ------------ - There may be (very) subtle issues with tracing on Python 3.12, which has redesigned the entire tracing infrastructure. 3.0.0a1 (2023-06-21) ==================== - Build binary wheels for S390x Linux. See `PR 358 <https://github.com/python-greenlet/greenlet/pull/358>`_ from Steven Silvester. - Fix a rare crash on shutdown seen in uWSGI deployments. See `issue 330 <https://github.com/python-greenlet/greenlet/issues/330>`_ and `PR 356 <https://github.com/python-greenlet/greenlet/pull/356>`_ from Andrew Wason. - Make the platform-specific low-level C/assembly snippets stop using the ``register`` storage class. Newer versions of standards remove this storage class, and it has been generally ignored by many compilers for some time. See `PR 347 <https://github.com/python-greenlet/greenlet/pull/347>`_ from Khem Raj. - Add initial support for Python 3.12. See `issue <https://github.com/python-greenlet/greenlet/issues/323>`_ and `PR <https://github.com/python-greenlet/greenlet/pull/327>`_; thanks go to (at least) Michael Droettboom, Andreas Motl, Thomas A Caswell, raphaelauv, Hugo van Kemenade, Mark Shannon, and Petr Viktorin. - Remove support for end-of-life Python versions, including Python 2.7, Python 3.5 and Python 3.6. - Require a compiler that supports ``noinline`` directives. See `issue 271 <https://github.com/python-greenlet/greenlet/issues/266>`_. - Require a compiler that supports C++11.
No description provided.