Skip to content

Commit

Permalink
Fix accessing greenlet.gr_frame.f_back on Python 3.12.
Browse files Browse the repository at this point in the history
Previously it crashed.
  • Loading branch information
jamadden committed Sep 1, 2023
1 parent 0582b89 commit 40646dc
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 9 deletions.
17 changes: 11 additions & 6 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@
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.
- 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
Expand Down Expand Up @@ -162,7 +167,7 @@
====================

Platforms
---------
~~~~~~~~~

- Add experimental, untested support for 64-bit Windows on ARM using
MSVC. See `PR 271 <https://github.com/python-greenlet/greenlet/pull/271>`_.
Expand All @@ -185,7 +190,7 @@ Platforms
thanks to Brandt Bucher and the CPython developers.

Fixes
-----
~~~~~

- Fix several leaks that could occur when using greenlets from
multiple threads. For example, it is no longer necessary to call
Expand All @@ -205,7 +210,7 @@ Fixes
platforms. This work is ongoing.

Changes
-------
~~~~~~~

- The repr of some greenlets has changed. In particular, if the
greenlet object was running in a thread that has exited, the repr
Expand Down Expand Up @@ -312,7 +317,7 @@ Changes
- (Documentation) Publish the change log to https://greenlet.readthedocs.io

Supported Platforms
-------------------
~~~~~~~~~~~~~~~~~~~

- Drop support for Python 2.4, 2.5, 2.6, 3.0, 3.1, 3.2 and 3.4.
The project metadata now includes the ``python_requires`` data to
Expand All @@ -322,7 +327,7 @@ Supported Platforms
<https://github.com/python-greenlet/greenlet/pull/197>`_.

Packaging Changes
-----------------
~~~~~~~~~~~~~~~~~

- Require setuptools to build from source.
- Stop asking setuptools to build both .tar.gz and .zip
Expand Down
5 changes: 4 additions & 1 deletion src/greenlet/greenlet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2209,7 +2209,9 @@ green_switch(PyGreenlet* self, PyObject* args, PyObject* kwargs)
// It's possible it's never been switched to.
assert(!current->args());
#endif
return result.relinquish_ownership();
PyObject* p = result.relinquish_ownership();
assert(p || PyErr_Occurred());
return p;
}
catch(const PyErrOccurred&) {
return nullptr;
Expand Down Expand Up @@ -2760,6 +2762,7 @@ static PyMethodDef green_methods[] = {
};

static PyGetSetDef green_getsets[] = {
/* name, getter, setter, doc, context pointer */
{"__dict__", (getter)green_getdict, (setter)green_setdict, /*XXX*/ NULL},
{"run", (getter)green_getrun, (setter)green_setrun, /*XXX*/ NULL},
{"parent", (getter)green_getparent, (setter)green_setparent, /*XXX*/ NULL},
Expand Down
6 changes: 6 additions & 0 deletions src/greenlet/greenlet_cpython_compat.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,15 @@ Greenlet won't compile on anything older than Python 3.11 alpha 4 (see
#ifndef _Py_DEC_REFTOTAL
/* _Py_DEC_REFTOTAL macro has been removed from Python 3.9 by:
https://github.com/python/cpython/commit/49932fec62c616ec88da52642339d83ae719e924
The symbol we use to replace it was removed by at least 3.12.
*/
# ifdef Py_REF_DEBUG
# if GREENLET_PY312
# define _Py_DEC_REFTOTAL
# else
# define _Py_DEC_REFTOTAL _Py_RefTotal--
# endif
# else
# define _Py_DEC_REFTOTAL
# endif
Expand Down
27 changes: 25 additions & 2 deletions src/greenlet/greenlet_greenlet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ using greenlet::refs::BorrowedGreenlet;
# define _PyInterpreterFrame _interpreter_frame
#endif

#if GREENLET_PY312
# include "internal/pycore_frame.h"
#endif

// XXX: TODO: Work to remove all virtual functions
// for speed of calling and size of objects (no vtable).
// One pattern is the Curiously Recurring Template
Expand Down Expand Up @@ -113,6 +117,9 @@ namespace greenlet
PyObject** datastack_top;
PyObject** datastack_limit;
#endif
#if GREENLET_PY312
_PyInterpreterFrame* _prev_frame;
#endif

public:
PythonState();
Expand Down Expand Up @@ -721,6 +728,9 @@ PythonState::PythonState()
,datastack_top(nullptr)
,datastack_limit(nullptr)
#endif
#if GREENLET_PY312
,_prev_frame(nullptr)
#endif
{
#if GREENLET_USE_CFRAME
/*
Expand Down Expand Up @@ -809,6 +819,12 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
PyFrameObject *frame = PyThreadState_GetFrame((PyThreadState *)tstate);
Py_XDECREF(frame); // PyThreadState_GetFrame gives us a new reference.
this->_top_frame.steal(frame);
#if GREENLET_PY312
if (frame) {
this->_prev_frame = frame->f_frame->previous;
frame->f_frame->previous = nullptr;
}
#endif
#if GREENLET_PY312
this->trash_delete_nesting = tstate->trash.delete_nesting;
#else // not 312
Expand Down Expand Up @@ -843,9 +859,16 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
#if GREENLET_PY312
tstate->py_recursion_remaining = tstate->py_recursion_limit - this->py_recursion_depth;
tstate->c_recursion_remaining = C_RECURSION_LIMIT - this->c_recursion_depth;
#else // not 3.12
// We're just going to throw this object away anyway, go ahead and
// do it now.
PyFrameObject* frame = this->_top_frame.relinquish_ownership();
if (frame && frame->f_frame) {
frame->f_frame->previous = this->_prev_frame;
}
this->_prev_frame = nullptr;
#else // \/ 3.11
tstate->recursion_remaining = tstate->recursion_limit - this->recursion_depth;
#endif
#endif // GREENLET_PY312
tstate->cframe->current_frame = this->current_frame;
tstate->datastack_chunk = this->datastack_chunk;
tstate->datastack_top = this->datastack_top;
Expand Down
19 changes: 19 additions & 0 deletions src/greenlet/tests/test_greenlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,25 @@ def test_switch_to_dead_greenlet_reparent(self):
self.assertIsNone(result)
self.assertEqual(seen, [42, 24])

def test_can_access_f_back_of_suspended_greenlet(self):
# On Python 3.12, they added a ->previous field to
# _PyInterpreterFrame that has to be cleared when a frame is inactive.
# If we got that wrong, this immediately crashes.
main = greenlet.getcurrent()

def Hub():
main.switch()

hub = greenlet(Hub)
# start it
hub.switch()
# now it is suspended
self.assertIsNotNone(hub.gr_frame)
# The next line is what would crash
self.assertIsNone(hub.gr_frame.f_back)




class TestGreenletSetParentErrors(TestCase):
def test_threaded_reparent(self):
Expand Down
5 changes: 5 additions & 0 deletions src/greenlet/tests/test_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,8 @@ def g2_run():
('call', '__exit__'),
('c_call', '__exit__'),
])


if __name__ == '__main__':
import unittest
unittest.main()

0 comments on commit 40646dc

Please sign in to comment.