Skip to content

Commit 44085a3

Browse files
authored
bpo-42990: Refactor _PyFrame_New_NoTrack() (GH-24566)
* Refactor _PyFrame_New_NoTrack() and PyFunction_NewWithQualName() code. * PyFrame_New() checks for _PyEval_BuiltinsFromGlobals() failure. * Fix a ref leak in _PyEval_BuiltinsFromGlobals() error path. * Complete PyFunction_GetModule() documentation: it returns a borrowed reference and it can return NULL. * Move _PyEval_BuiltinsFromGlobals() definition to the internal C API. * PyFunction_NewWithQualName() uses _Py_IDENTIFIER() API for the "__name__" string to make it compatible with subinterpreters.
1 parent cc96231 commit 44085a3

File tree

6 files changed

+114
-112
lines changed

6 files changed

+114
-112
lines changed

Doc/c-api/function.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ There are a few functions specific to Python functions.
6161
6262
.. c:function:: PyObject* PyFunction_GetModule(PyObject *op)
6363
64-
Return the *__module__* attribute of the function object *op*. This is normally
65-
a string containing the module name, but can be set to any other object by
66-
Python code.
64+
Return a :term:`borrowed reference` to the *__module__* attribute of the
65+
function object *op*. It can be *NULL*.
66+
67+
This is normally a string containing the module name, but can be set to any
68+
other object by Python code.
6769
6870
6971
.. c:function:: PyObject* PyFunction_GetDefaults(PyObject *op)

Include/cpython/frameobject.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,3 @@ PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *);
9292
PyAPI_FUNC(void) _PyFrame_DebugMallocStats(FILE *out);
9393

9494
PyAPI_FUNC(PyFrameObject *) PyFrame_GetBack(PyFrameObject *frame);
95-
96-
PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals);

Include/internal/pycore_ceval.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,12 @@ PyAPI_FUNC(void) _PyEval_SetCoroutineOriginTrackingDepth(
3131
PyThreadState *tstate,
3232
int new_depth);
3333

34-
/* Private function */
3534
void _PyEval_Fini(void);
3635

36+
37+
extern PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals);
38+
39+
3740
static inline PyObject*
3841
_PyEval_EvalFrame(PyThreadState *tstate, PyFrameObject *f, int throwflag)
3942
{

Objects/frameobject.c

Lines changed: 43 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
/* Frame object implementation */
22

33
#include "Python.h"
4-
#include "pycore_object.h"
5-
#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED()
4+
#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals()
5+
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
66

7-
#include "code.h"
8-
#include "frameobject.h"
9-
#include "opcode.h"
7+
#include "frameobject.h" // PyFrameObject
8+
#include "opcode.h" // EXTENDED_ARG
109
#include "structmember.h" // PyMemberDef
1110

1211
#define OFF(x) offsetof(PyFrameObject, x)
@@ -762,9 +761,7 @@ _Py_IDENTIFIER(__builtins__);
762761
static inline PyFrameObject*
763762
frame_alloc(PyCodeObject *code)
764763
{
765-
PyFrameObject *f;
766-
767-
f = code->co_zombieframe;
764+
PyFrameObject *f = code->co_zombieframe;
768765
if (f != NULL) {
769766
code->co_zombieframe = NULL;
770767
_Py_NewReference((PyObject *)f);
@@ -803,57 +800,45 @@ frame_alloc(PyCodeObject *code)
803800
_Py_NewReference((PyObject *)f);
804801
}
805802

806-
f->f_code = code;
807803
extras = code->co_nlocals + ncells + nfrees;
808804
f->f_valuestack = f->f_localsplus + extras;
809-
for (Py_ssize_t i=0; i<extras; i++) {
805+
for (Py_ssize_t i=0; i < extras; i++) {
810806
f->f_localsplus[i] = NULL;
811807
}
812-
f->f_locals = NULL;
813-
f->f_trace = NULL;
814808
return f;
815809
}
816810

817811

818812
PyFrameObject* _Py_HOT_FUNCTION
819813
_PyFrame_New_NoTrack(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals)
820814
{
821-
#ifdef Py_DEBUG
822-
if (con == NULL || con->fc_code == NULL ||
823-
(locals != NULL && !PyMapping_Check(locals))) {
824-
PyErr_BadInternalCall();
825-
return NULL;
826-
}
827-
#endif
828-
829-
PyFrameObject *back = tstate->frame;
815+
assert(con != NULL);
816+
assert(con->fc_globals != NULL);
817+
assert(con->fc_builtins != NULL);
818+
assert(con->fc_code != NULL);
819+
assert(locals == NULL || PyMapping_Check(locals));
830820

831821
PyFrameObject *f = frame_alloc((PyCodeObject *)con->fc_code);
832822
if (f == NULL) {
833823
return NULL;
834824
}
835825

826+
f->f_back = (PyFrameObject*)Py_XNewRef(tstate->frame);
827+
f->f_code = (PyCodeObject *)Py_NewRef(con->fc_code);
828+
f->f_builtins = Py_NewRef(con->fc_builtins);
829+
f->f_globals = Py_NewRef(con->fc_globals);
830+
f->f_locals = Py_XNewRef(locals);
831+
// f_valuestack initialized by frame_alloc()
832+
f->f_trace = NULL;
836833
f->f_stackdepth = 0;
837-
Py_INCREF(con->fc_builtins);
838-
f->f_builtins = con->fc_builtins;
839-
Py_XINCREF(back);
840-
f->f_back = back;
841-
Py_INCREF(con->fc_code);
842-
Py_INCREF(con->fc_globals);
843-
f->f_globals = con->fc_globals;
844-
Py_XINCREF(locals);
845-
f->f_locals = locals;
846-
834+
f->f_trace_lines = 1;
835+
f->f_trace_opcodes = 0;
836+
f->f_gen = NULL;
847837
f->f_lasti = -1;
848838
f->f_lineno = 0;
849839
f->f_iblock = 0;
850840
f->f_state = FRAME_CREATED;
851-
f->f_gen = NULL;
852-
f->f_trace_opcodes = 0;
853-
f->f_trace_lines = 1;
854-
855-
assert(f->f_code != NULL);
856-
841+
// f_blockstack and f_localsplus initialized by frame_alloc()
857842
return f;
858843
}
859844

@@ -863,6 +848,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
863848
PyObject *globals, PyObject *locals)
864849
{
865850
PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
851+
if (builtins == NULL) {
852+
return NULL;
853+
}
866854
PyFrameConstructor desc = {
867855
.fc_globals = globals,
868856
.fc_builtins = builtins,
@@ -875,8 +863,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
875863
};
876864
PyFrameObject *f = _PyFrame_New_NoTrack(tstate, &desc, locals);
877865
Py_DECREF(builtins);
878-
if (f)
866+
if (f) {
879867
_PyObject_GC_TRACK(f);
868+
}
880869
return f;
881870
}
882871

@@ -1173,27 +1162,30 @@ PyFrame_GetBack(PyFrameObject *frame)
11731162
return back;
11741163
}
11751164

1176-
PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals) {
1165+
PyObject*
1166+
_PyEval_BuiltinsFromGlobals(PyObject *globals)
1167+
{
11771168
PyObject *builtins = _PyDict_GetItemIdWithError(globals, &PyId___builtins__);
11781169
if (builtins) {
11791170
if (PyModule_Check(builtins)) {
11801171
builtins = PyModule_GetDict(builtins);
11811172
assert(builtins != NULL);
11821173
}
1174+
return Py_NewRef(builtins);
11831175
}
1176+
1177+
if (PyErr_Occurred()) {
1178+
return NULL;
1179+
}
1180+
1181+
/* No builtins! Make up a minimal one. Give them 'None', at least. */
1182+
builtins = PyDict_New();
11841183
if (builtins == NULL) {
1185-
if (PyErr_Occurred()) {
1186-
return NULL;
1187-
}
1188-
/* No builtins! Make up a minimal one
1189-
Give them 'None', at least. */
1190-
builtins = PyDict_New();
1191-
if (builtins == NULL ||
1192-
PyDict_SetItemString(
1193-
builtins, "None", Py_None) < 0)
1194-
return NULL;
1184+
return NULL;
1185+
}
1186+
if (PyDict_SetItemString(builtins, "None", Py_None) < 0) {
1187+
Py_DECREF(builtins);
1188+
return NULL;
11951189
}
1196-
else
1197-
Py_INCREF(builtins);
11981190
return builtins;
11991191
}

Objects/funcobject.c

Lines changed: 59 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -2,83 +2,90 @@
22
/* Function object implementation */
33

44
#include "Python.h"
5-
#include "pycore_object.h"
6-
#include "frameobject.h"
7-
#include "code.h"
5+
#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals()
6+
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
87
#include "structmember.h" // PyMemberDef
98

109
PyObject *
1110
PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname)
1211
{
13-
PyFunctionObject *op;
14-
PyObject *doc, *consts, *module;
15-
static PyObject *__name__ = NULL;
12+
assert(globals != NULL);
13+
assert(PyDict_Check(globals));
14+
Py_INCREF(globals);
1615

17-
if (__name__ == NULL) {
18-
__name__ = PyUnicode_InternFromString("__name__");
19-
if (__name__ == NULL)
20-
return NULL;
16+
PyCodeObject *code_obj = (PyCodeObject *)code;
17+
Py_INCREF(code_obj);
18+
19+
PyObject *name = code_obj->co_name;
20+
assert(name != NULL);
21+
Py_INCREF(name);
22+
if (!qualname) {
23+
qualname = name;
2124
}
25+
Py_INCREF(qualname);
2226

23-
/* __module__: If module name is in globals, use it.
24-
Otherwise, use None. */
25-
module = PyDict_GetItemWithError(globals, __name__);
26-
if (module) {
27-
Py_INCREF(module);
27+
PyObject *consts = code_obj->co_consts;
28+
assert(PyTuple_Check(consts));
29+
PyObject *doc;
30+
if (PyTuple_Size(consts) >= 1) {
31+
doc = PyTuple_GetItem(consts, 0);
32+
if (!PyUnicode_Check(doc)) {
33+
doc = Py_None;
34+
}
2835
}
29-
else if (PyErr_Occurred()) {
30-
return NULL;
36+
else {
37+
doc = Py_None;
38+
}
39+
Py_INCREF(doc);
40+
41+
// __module__: Use globals['__name__'] if it exists, or NULL.
42+
_Py_IDENTIFIER(__name__);
43+
PyObject *module = _PyDict_GetItemIdWithError(globals, &PyId___name__);
44+
PyObject *builtins = NULL;
45+
if (module == NULL && PyErr_Occurred()) {
46+
goto error;
47+
}
48+
Py_XINCREF(module);
49+
50+
builtins = _PyEval_BuiltinsFromGlobals(globals);
51+
if (builtins == NULL) {
52+
goto error;
3153
}
3254

33-
op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type);
55+
PyFunctionObject *op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type);
3456
if (op == NULL) {
35-
Py_XDECREF(module);
36-
return NULL;
57+
goto error;
3758
}
3859
/* Note: No failures from this point on, since func_dealloc() does not
3960
expect a partially-created object. */
4061

41-
op->func_weakreflist = NULL;
42-
Py_INCREF(code);
43-
op->func_code = code;
44-
assert(globals != NULL);
45-
Py_INCREF(globals);
4662
op->func_globals = globals;
47-
PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
48-
if (builtins == NULL) {
49-
return NULL;
50-
}
5163
op->func_builtins = builtins;
52-
op->func_name = ((PyCodeObject *)code)->co_name;
53-
Py_INCREF(op->func_name);
54-
op->func_defaults = NULL; /* No default arguments */
55-
op->func_kwdefaults = NULL; /* No keyword only defaults */
64+
op->func_name = name;
65+
op->func_qualname = qualname;
66+
op->func_code = (PyObject*)code_obj;
67+
op->func_defaults = NULL; // No default positional arguments
68+
op->func_kwdefaults = NULL; // No default keyword arguments
5669
op->func_closure = NULL;
57-
op->vectorcall = _PyFunction_Vectorcall;
58-
op->func_module = module;
59-
60-
consts = ((PyCodeObject *)code)->co_consts;
61-
if (PyTuple_Size(consts) >= 1) {
62-
doc = PyTuple_GetItem(consts, 0);
63-
if (!PyUnicode_Check(doc))
64-
doc = Py_None;
65-
}
66-
else
67-
doc = Py_None;
68-
Py_INCREF(doc);
6970
op->func_doc = doc;
70-
7171
op->func_dict = NULL;
72+
op->func_weakreflist = NULL;
73+
op->func_module = module;
7274
op->func_annotations = NULL;
73-
74-
if (qualname)
75-
op->func_qualname = qualname;
76-
else
77-
op->func_qualname = op->func_name;
78-
Py_INCREF(op->func_qualname);
75+
op->vectorcall = _PyFunction_Vectorcall;
7976

8077
_PyObject_GC_TRACK(op);
8178
return (PyObject *)op;
79+
80+
error:
81+
Py_DECREF(globals);
82+
Py_DECREF(code_obj);
83+
Py_DECREF(name);
84+
Py_DECREF(qualname);
85+
Py_DECREF(doc);
86+
Py_XDECREF(module);
87+
Py_XDECREF(builtins);
88+
return NULL;
8289
}
8390

8491
PyObject *

Python/ceval.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ PyEval_EvalCode(PyObject *co, PyObject *globals, PyObject *locals)
908908
.fc_closure = NULL
909909
};
910910
PyThreadState *tstate = PyThreadState_GET();
911-
PyObject *res =_PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
911+
PyObject *res = _PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
912912
Py_DECREF(builtins);
913913
return res;
914914
}
@@ -4800,8 +4800,8 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals,
48004800
};
48014801
PyThreadState *tstate = _PyThreadState_GET();
48024802
res = _PyEval_Vector(tstate, &constr, locals,
4803-
allargs, argcount,
4804-
kwnames);
4803+
allargs, argcount,
4804+
kwnames);
48054805
if (kwcount) {
48064806
Py_DECREF(kwnames);
48074807
PyMem_Free(newargs);

0 commit comments

Comments
 (0)