Skip to content

gh-113753: Clear finalized bit when allocating PyAsyncGenASend from free-list #113754

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jan 10, 2024
Merged

gh-113753: Clear finalized bit when allocating PyAsyncGenASend from free-list #113754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ae71e9
gh-113753: Clear finalized bit when allocating PyAsyncGenASend from f…
colesbury Jan 5, 2024
fba7b02
Add NEWS
colesbury Jan 5, 2024
3856b92
Merge branch 'main' into gh-113753-asend
gvanrossum Jan 10, 2024
fe24eec
Move _PyGC_CLEAR_FINALIZED call
colesbury Jan 10, 2024
614ac7b
Fix location of _PyGC_CLEAR_FINALIZED call and add test
colesbury Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Include/internal/pycore_gc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ static inline void _PyGC_SET_FINALIZED(PyObject *op) {
PyGC_Head *gc = _Py_AS_GC(op);
_PyGCHead_SET_FINALIZED(gc);
}
static inline void _PyGC_CLEAR_FINALIZED(PyObject *op) {
PyGC_Head *gc = _Py_AS_GC(op);
gc->_gc_prev &= ~_PyGC_PREV_MASK_FINALIZED;
}

Comment on lines +125 to 129
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is expedient, but why not follow the patter used by FINALIZED and SET_FINALIZED that have a helper that skips the _Py_AS_GC() call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those functions (_PyGCHead_SET_FINALIZED and _PyGCHead_FINALIZED) are likely to be removed in other PRs as I work on making the GC thread-safe in free-threaded builds. It's easier to support both builds when the functions take PyObject* instead of PyGC_Head* because the free-threaded builds will store the GC flags on the PyObject* itself (and eventually may not even have a PyGC_Head prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also OK with following the existing pattern for now and dealing with any potential removal later if you prefer.

Mostly I'm stuck on the warning generated in test_async_gen_exception_12. I don't know how to deal with it. Maybe @kumaraditya303 has suggestions?


/* GC runtime state */
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_asyncgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,14 @@ def test_asend(self):
async def gen():
yield 1

# gh-113753: asend objects allocated from a free-list should warn.
# Ensure there is a finalized 'asend' object ready to be reused.
try:
g = gen()
g.asend(None).send(None)
except StopIteration:
pass

msg = f"coroutine method 'asend' of '{gen.__qualname__}' was never awaited"
with self.assertWarnsRegex(RuntimeWarning, msg):
g = gen()
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2024-01-05-21-28-48.gh-issue-113753.2HNiuq.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix an issue where the finalizer of ``PyAsyncGenASend`` objects might not be
called if they were allocated from a free list.
2 changes: 2 additions & 0 deletions Objects/genobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _PyEval_EvalFrame()
#include "pycore_frame.h" // _PyInterpreterFrame
#include "pycore_gc.h" // _PyGC_CLEAR_FINALIZED()
#include "pycore_genobject.h" // struct _Py_async_gen_state
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
Expand Down Expand Up @@ -1739,6 +1740,7 @@ async_gen_asend_dealloc(PyAsyncGenASend *o)
#endif
if (state->asend_numfree < _PyAsyncGen_MAXFREELIST) {
assert(PyAsyncGenASend_CheckExact(o));
_PyGC_CLEAR_FINALIZED((PyObject *)o);
state->asend_freelist[state->asend_numfree++] = o;
}
else
Expand Down