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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if there is a trivial way for a generator to hold onto a borrowed reference since for a generator g, you can always access the code object by doing g.gi_code. The function can get freed but the code object is still needed. See, for example, this test:

def test_generator_resurrect(self):
# Test that a resurrected generator still has a valid gi_code
resurrected = []
# Resurrect a generator in a finalizer
exec(textwrap.dedent("""
def gen():
try:
yield
except:
resurrected.append(g)
g = gen()
next(g)
"""), {"resurrected": resurrected})
support.gc_collect()
self.assertEqual(len(resurrected), 1)
self.assertIsInstance(resurrected[0].gi_code, types.CodeType)

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
29 changes: 29 additions & 0 deletions Lib/test/test_ctypes/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,35 @@ def callback(*args):
callback = proto(callback)
self.assertRaises(ArgumentError, lambda: callback((1, 2, 3, 4), POINT()))

def test_reassign_code_while_running(self):
freevar1 = None
freevar2 = None

def replacement():
freevar1
freevar2
return "replacement"

def original():
original.__code__ = replacement.__code__
return "original"

self.assertEqual(original(), "original")
self.assertEqual(original(), "replacement")

def test_function_code_object_memory_leak(self):
def get_function(i):
ns = {}
exec(f"def f(): return {i}", ns)
return ns["f"]

def f(): pass

for i in range(1000):
f.__code__ = get_function(i).__code__

self.assertEqual(f(), 999)


if __name__ == '__main__':
unittest.main()
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.

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
17 changes: 8 additions & 9 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1870,7 +1870,7 @@ dummy_func(
/* We don't know which of these is relevant here, so keep them equal */
assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
#if TIER_ONE && defined(Py_DEBUG)
if (!PyStackRef_IsNone(frame->f_executable)) {
if (frame->f_executable != NULL) {
Py_ssize_t i = frame->instr_ptr - _PyFrame_GetBytecode(frame);
assert(i >= 0 && i <= INT_MAX);
int opcode = _Py_GetBaseCodeUnit(_PyFrame_GetCode(frame), (int)i).op.code;
Expand Down Expand Up @@ -5854,10 +5854,9 @@ dummy_func(
SAVE_STACK();
_PyInterpreterFrame *gen_frame = &gen->gi_iframe;
frame->instr_ptr++;
_PyFrame_Copy(frame, gen_frame);
_PyFrame_CopyForGenerators(frame, gen_frame);
assert(frame->frame_obj == NULL);
gen->gi_frame_state = FRAME_CREATED;
gen_frame->owner = FRAME_OWNED_BY_GENERATOR;
_Py_LeaveRecursiveCallPy(tstate);
_PyInterpreterFrame *prev = frame->previous;
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, prev);
Expand Down Expand Up @@ -6318,15 +6317,15 @@ dummy_func(
}

tier2 op(_GUARD_CODE_VERSION__PUSH_FRAME, (version/2 -- )) {
PyObject *code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
PyObject *code = frame->f_executable;
assert(PyCode_Check(code));
if (((PyCodeObject *)code)->co_version != version) {
EXIT_IF(true);
}
}

tier2 op(_GUARD_CODE_VERSION_YIELD_VALUE, (version/2 -- )) {
PyObject *code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
PyObject *code = frame->f_executable;
assert(PyCode_Check(code));
if (((PyCodeObject *)code)->co_version != version) {
frame->instr_ptr += 1 + INLINE_CACHE_ENTRIES_SEND;
Expand All @@ -6335,7 +6334,7 @@ dummy_func(
}

tier2 op(_GUARD_CODE_VERSION_RETURN_VALUE, (version/2 -- )) {
PyObject *code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
PyObject *code = frame->f_executable;
assert(PyCode_Check(code));
if (((PyCodeObject *)code)->co_version != version) {
frame->instr_ptr += frame->return_offset;
Expand All @@ -6344,7 +6343,7 @@ dummy_func(
}

tier2 op(_GUARD_CODE_VERSION_RETURN_GENERATOR, (version/2 -- )) {
PyObject *code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
PyObject *code = frame->f_executable;
assert(PyCode_Check(code));
if (((PyCodeObject *)code)->co_version != version) {
frame->instr_ptr += frame->return_offset;
Expand Down Expand Up @@ -6638,14 +6637,14 @@ dummy_func(
}
tracer->prev_state.recorded_count = 0;
tracer->prev_state.instr = next_instr;
PyObject *prev_code = PyStackRef_AsPyObjectBorrow(frame->f_executable);
PyObject *prev_code = frame->f_executable;
if (tracer->prev_state.instr_code != (PyCodeObject *)prev_code) {
Py_SETREF(tracer->prev_state.instr_code, (PyCodeObject*)Py_NewRef((prev_code)));
}

tracer->prev_state.instr_frame = frame;
tracer->prev_state.instr_oparg = oparg;
tracer->prev_state.instr_stacklevel = PyStackRef_IsNone(frame->f_executable) ? 2 : STACK_LEVEL();
tracer->prev_state.instr_stacklevel = frame->f_executable == NULL ? 2 : STACK_LEVEL();
if (_PyOpcode_Caches[_PyOpcode_Deopt[opcode]]
// Branch opcodes use the cache for branch history, not
// specialization counters. Don't reset it.
Expand Down
4 changes: 2 additions & 2 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
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 + 1;
entry.frame.stackpointer = entry.stack;
entry.frame.owner = FRAME_OWNED_BY_INTERPRETER;
Expand Down Expand Up @@ -1982,7 +1982,7 @@ clear_thread_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
tstate->datastack_top);
assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame);
_PyFrame_ClearExceptCode(frame);
PyStackRef_CLEAR(frame->f_executable);
frame->f_executable = NULL;
_PyThreadState_PopFrame(tstate, frame);
}

Expand Down
Loading
Loading