Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Include/cpython/funcobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ typedef struct {
PyObject *func_annotations; /* Annotations, a dict or NULL */
PyObject *func_annotate; /* Callable to fill the annotations dictionary */
PyObject *func_typeparams; /* Tuple of active type variables or NULL */
PyObject *func_old_codes; /* List of past code objects or NULL */
vectorcallfunc vectorcall;
/* Version number for use by specializer.
* Can set to non-zero when we want to specialize.
Expand Down
26 changes: 17 additions & 9 deletions Include/internal/pycore_interpframe.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ extern "C" {
((int)((IF)->instr_ptr - _PyFrame_GetBytecode((IF))))

static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
assert(!PyStackRef_IsNull(f->f_executable));
PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable);
assert(PyCode_Check(executable));
return (PyCodeObject *)executable;
assert(f->f_executable != NULL);
assert(PyCode_Check(f->f_executable));
return (PyCodeObject *)f->f_executable;
}

// Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or
Expand All @@ -36,15 +35,15 @@ _PyFrame_SafeGetCode(_PyInterpreterFrame *f)
return NULL;
}

if (PyStackRef_IsNull(f->f_executable)) {
if (f->f_executable == NULL) {
return NULL;
}
void *ptr;
memcpy(&ptr, &f->f_executable, sizeof(f->f_executable));
if (_PyMem_IsPtrFreed(ptr)) {
return NULL;
}
PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable);
PyObject *executable = f->f_executable;
if (_PyObject_IsFreed(executable)) {
return NULL;
}
Expand Down Expand Up @@ -132,7 +131,7 @@ _PyFrame_NumSlotsForCodeObject(PyCodeObject *code)

static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest)
{
dest->f_executable = PyStackRef_MakeHeapSafe(src->f_executable);
dest->f_executable = src->f_executable;
// Don't leave a dangling pointer to the old frame when creating generators
// and coroutines:
dest->previous = NULL;
Expand Down Expand Up @@ -160,6 +159,15 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
}
}

/* Generator frames need a strong reference to the code object */
static inline void
_PyFrame_CopyForGenerators(_PyInterpreterFrame *old_frame, _PyInterpreterFrame *gen_frame)
{
_PyFrame_Copy(old_frame, gen_frame);
gen_frame->owner = FRAME_OWNED_BY_GENERATOR;
Py_INCREF(gen_frame->f_executable);
}

#ifdef Py_GIL_DISABLED
static inline void
_PyFrame_InitializeTLBC(PyThreadState *tstate, _PyInterpreterFrame *frame,
Expand Down Expand Up @@ -191,7 +199,7 @@ _PyFrame_Initialize(
{
frame->previous = previous;
frame->f_funcobj = func;
frame->f_executable = PyStackRef_FromPyObjectNew(code);
frame->f_executable = (PyObject *)code;
PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func);
frame->f_builtins = func_obj->func_builtins;
frame->f_globals = func_obj->func_globals;
Expand Down Expand Up @@ -424,7 +432,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
assert(tstate->datastack_top < tstate->datastack_limit);
frame->previous = previous;
frame->f_funcobj = PyStackRef_None;
frame->f_executable = PyStackRef_FromPyObjectNew(code);
frame->f_executable = (PyObject *)code;
#ifdef Py_DEBUG
frame->f_builtins = NULL;
frame->f_globals = NULL;
Expand Down
4 changes: 3 additions & 1 deletion Include/internal/pycore_interpframe_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ enum _frameowner {
};

struct _PyInterpreterFrame {
_PyStackRef f_executable; /* Deferred or strong reference (code object or None) */
/* Borrowed reference (code object or None) */
/* Strong reference for generators */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be a borrowed reference for generators as well.
The function object still keeps the code object alive, regardless of whether it is a generator function or not.

PyObject *f_executable;
struct _PyInterpreterFrame *previous;
_PyStackRef f_funcobj; /* Deferred or strong reference. Only valid if not on C stack */
PyObject *f_globals; /* Borrowed reference. Only valid if not on C stack */
Expand Down
1 change: 0 additions & 1 deletion Include/internal/pycore_stackref.h
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@ PyStackRef_TYPE(_PyStackRef stackref) {
STACKREF_CHECK_FUNC(Gen)
STACKREF_CHECK_FUNC(Bool)
STACKREF_CHECK_FUNC(ExceptionInstance)
STACKREF_CHECK_FUNC(Code)
STACKREF_CHECK_FUNC(Function)

static inline bool
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_capi/test_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,28 @@ def annofn(arg: int) -> str:
with self.assertRaises(SystemError):
_testcapi.function_get_annotations(None)

def test_function_old_codes(self):
def f():
pass

def g():
pass

def h():
pass

old_codes = _testcapi.function_get_old_codes(f)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The old codes list is an implementation detail.
Would it be possible to check the behavior rather than the implementation?

Specifically, test that:

  • reassigning the code object for a running function still works
  • reassigning many code objects doesn't leak memory

self.assertIsNone(old_codes)

f.__code__ = g.__code__
old_codes = _testcapi.function_get_old_codes(f)
self.assertIsInstance(old_codes, list)
self.assertEqual(len(old_codes), 1)

f.__code__ = h.__code__
old_codes = _testcapi.function_get_old_codes(f)
self.assertEqual(len(old_codes), 2)

# TODO: test PyFunction_New()
# TODO: test PyFunction_NewWithQualName()
# TODO: test PyFunction_SetVectorcall()
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,7 @@ def func():
check(x, size('3PiccPPP' + INTERPRETER_FRAME + 'P'))
# function
def func(): pass
check(func, size('16Pi'))
check(func, size('17Pi'))
class c():
@staticmethod
def foo():
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Avoid reference counting of :class:`code` objects when creating and destroying
frames by having functions retain a list of all past :class:`code` objects.

14 changes: 14 additions & 0 deletions Modules/_testcapi/function.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ function_get_annotations(PyObject *self, PyObject *func)
}


static PyObject *
function_get_old_codes(PyObject *self, PyObject *func)
{
PyFunctionObject *func_o = (PyFunctionObject *) func;
PyObject *old_codes = func_o->func_old_codes;
if (old_codes == NULL) {
Py_RETURN_NONE;
}

return Py_NewRef(old_codes);
}


static PyMethodDef test_methods[] = {
{"function_get_code", function_get_code, METH_O, NULL},
{"function_get_globals", function_get_globals, METH_O, NULL},
Expand All @@ -141,6 +154,7 @@ static PyMethodDef test_methods[] = {
{"function_get_closure", function_get_closure, METH_O, NULL},
{"function_set_closure", function_set_closure, METH_VARARGS, NULL},
{"function_get_annotations", function_get_annotations, METH_O, NULL},
{"function_get_old_codes", function_get_old_codes, METH_O, NULL},
{NULL},
};

Expand Down
2 changes: 1 addition & 1 deletion Modules/_testinternalcapi/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Test_EvalFrame(PyThreadState *tstate, _PyInterpreterFrame *frame, int throwflag)
entry.frame.f_globals = (PyObject*)0xaaa3;
entry.frame.f_builtins = (PyObject*)0xaaa4;
#endif
entry.frame.f_executable = PyStackRef_None;
entry.frame.f_executable = NULL;
entry.frame.instr_ptr = (_Py_CODEUNIT *)_Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS_PTR + 1;
entry.frame.stackpointer = entry.stack;
entry.frame.owner = FRAME_OWNED_BY_INTERPRETER;
Expand Down
11 changes: 5 additions & 6 deletions Modules/_testinternalcapi/test_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1939,7 +1939,7 @@ frame_dealloc(PyObject *op)

/* Kill all local variables including specials, if we own them */
if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) {
PyStackRef_CLEAR(frame->f_executable);
frame->f_executable = NULL;
PyStackRef_CLEAR(frame->f_funcobj);
Py_CLEAR(frame->f_locals);
_PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
Expand Down
15 changes: 15 additions & 0 deletions Objects/funcobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ _PyFunction_FromConstructor(PyFrameConstructor *constr)
op->func_typeparams = NULL;
op->vectorcall = _PyFunction_Vectorcall;
op->func_version = FUNC_VERSION_UNSET;
op->func_old_codes = NULL;
// NOTE: functions created via FrameConstructor do not use deferred
// reference counting because they are typically not part of cycles
// nor accessed by multiple threads.
Expand Down Expand Up @@ -223,6 +224,7 @@ PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname
op->func_typeparams = NULL;
op->vectorcall = _PyFunction_Vectorcall;
op->func_version = FUNC_VERSION_UNSET;
op->func_old_codes = NULL;
if (((code_obj->co_flags & CO_NESTED) == 0) ||
(code_obj->co_flags & CO_METHOD)) {
// Use deferred reference counting for top-level functions, but not
Expand Down Expand Up @@ -686,6 +688,17 @@ func_set_code(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))

handle_func_event(PyFunction_EVENT_MODIFY_CODE, op, value);
_PyFunction_ClearVersion(op);
if (op->func_old_codes == NULL) {
op->func_old_codes = PyList_New(0);
if (op->func_old_codes == NULL) {
return -1;
}
}

if (PyList_Append(op->func_old_codes, op->func_code) < 0) {
return -1;
}

Py_XSETREF(op->func_code, Py_NewRef(value));
return 0;
}
Expand Down Expand Up @@ -1114,6 +1127,7 @@ func_clear(PyObject *self)
Py_CLEAR(op->func_annotations);
Py_CLEAR(op->func_annotate);
Py_CLEAR(op->func_typeparams);
Py_CLEAR(op->func_old_codes);
// Don't Py_CLEAR(op->func_code), since code is always required
// to be non-NULL. Similarly, name and qualname shouldn't be NULL.
// However, name and qualname could be str subclasses, so they
Expand Down Expand Up @@ -1169,6 +1183,7 @@ func_traverse(PyObject *self, visitproc visit, void *arg)
Py_VISIT(f->func_annotate);
Py_VISIT(f->func_typeparams);
Py_VISIT(f->func_qualname);
Py_VISIT(f->func_old_codes);
return 0;
}

Expand Down
9 changes: 4 additions & 5 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ gen_traverse(PyObject *self, visitproc visit, void *arg)
else {
// We still need to visit the code object when the frame is cleared to
// ensure that it's kept alive if the reference is deferred.
_Py_VISIT_STACKREF(gen->gi_iframe.f_executable);
Py_VISIT(gen->gi_iframe.f_executable);
}
/* No need to visit cr_origin, because it's just tuples/str/int, so can't
participate in a reference cycle. */
Expand Down Expand Up @@ -231,7 +231,7 @@ gen_dealloc(PyObject *self)
gen_clear_frame(gen);
}
assert(gen->gi_exc_state.exc_value == NULL);
PyStackRef_CLEAR(gen->gi_iframe.f_executable);
Py_CLEAR(gen->gi_iframe.f_executable);
Py_CLEAR(gen->gi_name);
Py_CLEAR(gen->gi_qualname);

Expand Down Expand Up @@ -1105,7 +1105,7 @@ make_gen(PyTypeObject *type, PyFunctionObject *func)
gen->gi_weakreflist = NULL;
gen->gi_exc_state.exc_value = NULL;
gen->gi_exc_state.previous_item = NULL;
gen->gi_iframe.f_executable = PyStackRef_None;
gen->gi_iframe.f_executable = NULL;
assert(func->func_name != NULL);
gen->gi_name = Py_NewRef(func->func_name);
assert(func->func_qualname != NULL);
Expand Down Expand Up @@ -1176,11 +1176,10 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f,
assert(f->f_frame->frame_obj == NULL);
assert(f->f_frame->owner == FRAME_OWNED_BY_FRAME_OBJECT);
_PyInterpreterFrame *frame = &gen->gi_iframe;
_PyFrame_Copy((_PyInterpreterFrame *)f->_f_frame_data, frame);
_PyFrame_CopyForGenerators((_PyInterpreterFrame *)f->_f_frame_data, frame);
gen->gi_frame_state = FRAME_CREATED;
assert(frame->frame_obj == f);
f->f_frame = frame;
frame->owner = FRAME_OWNED_BY_GENERATOR;
assert(PyObject_GC_IsTracked((PyObject *)f));
Py_DECREF(f);
gen->gi_weakreflist = NULL;
Expand Down
Loading
Loading