Skip to content

Commit d9ad012

Browse files
DinoVDinoV
committed
Fix leaks and LOAD_ATTR specialization
1 parent 0c246bc commit d9ad012

File tree

10 files changed

+102
-46
lines changed

10 files changed

+102
-46
lines changed

‎Include/internal/pycore_dict.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ extern PyObject* _PyDict_GetItemIdWithError(PyObject *dp,
3838
extern int _PyDict_ContainsId(PyObject *, _Py_Identifier *);
3939
extern int _PyDict_SetItemId(PyObject *dp, _Py_Identifier *key, PyObject *item);
4040
extern int _PyDict_DelItemId(PyObject *mp, _Py_Identifier *key);
41+
extern void _PyDict_ClearKeysVersion(PyObject *mp);
4142

4243
extern int _PyDict_Next(
4344
PyObject *mp, Py_ssize_t *pos, PyObject **key, PyObject **value, Py_hash_t *hash);

‎Include/internal/pycore_moduleobject.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ extern Py_ssize_t _PyModule_GetFilenameUTF8(
5858
PyObject* _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress);
5959
PyObject* _Py_module_getattro(PyObject *m, PyObject *name);
6060

61+
PyAPI_FUNC(int) _PyModule_ReplaceLazyValue(PyObject *dict, PyObject *name, PyObject *value);
62+
6163
#ifdef __cplusplus
6264
}
6365
#endif

‎Objects/dictobject.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4719,6 +4719,14 @@ sizeof_lock_held(PyDictObject *mp)
47194719
return (Py_ssize_t)res;
47204720
}
47214721

4722+
void
4723+
_PyDict_ClearKeysVersion(PyObject *mp)
4724+
{
4725+
ASSERT_DICT_LOCKED(mp);
4726+
4727+
FT_ATOMIC_STORE_UINT32_RELAXED(((PyDictObject *)mp)->ma_keys->dk_version, 0);
4728+
}
4729+
47224730
Py_ssize_t
47234731
_PyDict_SizeOf(PyDictObject *mp)
47244732
{

‎Objects/moduleobject.c‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,23 @@ _PyModule_IsPossiblyShadowing(PyObject *origin)
10361036
return result;
10371037
}
10381038

1039+
int
1040+
_PyModule_ReplaceLazyValue(PyObject *dict, PyObject *name, PyObject *value)
1041+
{
1042+
// The adaptive interpreter uses the dictionary version to return the slot at
1043+
// a given index from the module. When replacing a value the version number doesn't
1044+
// change, so we need to atomically clear the version before replacing so that it
1045+
// doesn't return a lazy value.
1046+
int err;
1047+
Py_BEGIN_CRITICAL_SECTION(dict);
1048+
1049+
_PyDict_ClearKeysVersion(dict);
1050+
err = _PyDict_SetItem_LockHeld((PyDictObject *)dict, name, value);
1051+
1052+
Py_END_CRITICAL_SECTION();
1053+
return err;
1054+
}
1055+
10391056
PyObject*
10401057
_Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
10411058
{
@@ -1052,13 +1069,13 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
10521069
// instead treated as if the attribute doesn't exist.
10531070
PyErr_Clear();
10541071
}
1055-
1056-
return NULL;
1057-
} else if (PyDict_SetItem(m->md_dict, name, new_value) < 0) {
1058-
Py_DECREF(new_value);
10591072
Py_DECREF(attr);
10601073
return NULL;
10611074
}
1075+
1076+
if (_PyModule_ReplaceLazyValue(m->md_dict, name, new_value) < 0) {
1077+
Py_CLEAR(new_value);
1078+
}
10621079
Py_DECREF(attr);
10631080
return new_value;
10641081
}
@@ -1490,7 +1507,7 @@ module_get_dict(PyObject *mod, void *Py_UNUSED(ignored))
14901507
if (new_value == NULL) {
14911508
return NULL;
14921509
}
1493-
if (PyDict_SetItem(self->md_dict, key, new_value) < 0) {
1510+
if (_PyModule_ReplaceLazyValue(self->md_dict, key, new_value) < 0) {
14941511
Py_DECREF(new_value);
14951512
return NULL;
14961513
}

‎Python/bytecodes.c‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,12 +1799,13 @@ dummy_func(
17991799
ERROR_IF(v_o == NULL);
18001800
if (PyLazyImport_CheckExact(v_o)) {
18011801
PyObject *l_v = _PyImport_LoadLazyImportTstate(tstate, v_o);
1802-
if (l_v != NULL && PyDict_SetItem(GLOBALS(), name, l_v) < 0) {
1802+
Py_DECREF(v_o);
1803+
ERROR_IF(l_v == NULL);
1804+
int err = _PyModule_ReplaceLazyValue(GLOBALS(), name, l_v);
1805+
if (err < 0) {
18031806
JUMP_TO_LABEL(error);
18041807
}
1805-
Py_DECREF(v_o);
18061808
v_o = l_v;
1807-
ERROR_IF(v_o == NULL);
18081809
}
18091810

18101811
v = PyStackRef_FromPyObjectSteal(v_o);
@@ -1838,13 +1839,14 @@ dummy_func(
18381839
PyObject *res_o = PyStackRef_AsPyObjectBorrow(*res);
18391840
if (PyLazyImport_CheckExact(res_o)) {
18401841
PyObject *l_v = _PyImport_LoadLazyImportTstate(tstate, res_o);
1841-
if (l_v != NULL && PyDict_SetItem(GLOBALS(), name, l_v) < 0) {
1842+
PyStackRef_CLOSE(res[0]);
1843+
ERROR_IF(l_v == NULL);
1844+
int err = _PyModule_ReplaceLazyValue(GLOBALS(), name, l_v);
1845+
if (err < 0) {
1846+
Py_DECREF(l_v);
18421847
JUMP_TO_LABEL(error);
18431848
}
1844-
res_o = l_v;
1845-
PyStackRef_CLOSE(res[0]);
1846-
ERROR_IF(res_o == NULL);
1847-
*res = PyStackRef_FromPyObjectSteal(res_o);
1849+
*res = PyStackRef_FromPyObjectSteal(l_v);
18481850
}
18491851
}
18501852

‎Python/ceval.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3038,6 +3038,9 @@ check_lazy_import_comatibility(PyThreadState *tstate, PyObject *globals,
30383038
// Check if this module should be imported lazily due to the compatbility mode support via
30393039
// __lazy_modules__.
30403040
PyObject *lazy_modules = NULL;
3041+
PyObject *abs_name = NULL;
3042+
int res = -1;
3043+
30413044
if (globals != NULL &&
30423045
PyMapping_GetOptionalItem(globals, &_Py_ID(__lazy_modules__), &lazy_modules) < 0) {
30433046
return -1;
@@ -3047,15 +3050,19 @@ check_lazy_import_comatibility(PyThreadState *tstate, PyObject *globals,
30473050

30483051
int ilevel = PyLong_AsInt(level);
30493052
if (ilevel == -1 && _PyErr_Occurred(tstate)) {
3050-
return -1;
3053+
goto error;
30513054
}
30523055

3053-
PyObject *abs_name = _PyImport_GetAbsName(tstate, name, globals, ilevel);
3056+
abs_name = _PyImport_GetAbsName(tstate, name, globals, ilevel);
30543057
if (abs_name == NULL) {
3055-
return -1;
3058+
goto error;
30563059
}
30573060

3058-
return PySequence_Contains(lazy_modules, abs_name);
3061+
res = PySequence_Contains(lazy_modules, abs_name);
3062+
error:
3063+
Py_XDECREF(abs_name);
3064+
Py_XDECREF(lazy_modules);
3065+
return res;
30593066
}
30603067

30613068
PyObject *
@@ -3302,8 +3309,10 @@ _PyEval_LazyImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name)
33023309
PyObject *mod_dict = PyModule_GetDict(mod);
33033310
if (mod_dict != NULL) {
33043311
if (PyDict_GetItemRef(mod_dict, name, &ret) < 0) {
3312+
Py_DECREF(mod);
33053313
return NULL;
33063314
} else if (ret != NULL) {
3315+
Py_DECREF(mod);
33073316
return ret;
33083317
}
33093318
}

‎Python/executor_cases.c.h‎

Lines changed: 17 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h‎

Lines changed: 13 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/import.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3706,6 +3706,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
37063706
{
37073707
PyObject *obj = NULL;
37083708
PyObject *fromlist = NULL;
3709+
PyObject *import_func = NULL;
37093710
assert(lazy_import != NULL);
37103711
assert(PyLazyImport_CheckExact(lazy_import));
37113712

@@ -3723,7 +3724,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
37233724
}
37243725

37253726
int is_loading = PySet_Contains(importing, lazy_import);
3726-
if (is_loading < 0 ) {
3727+
if (is_loading < 0) {
37273728
return NULL;
37283729
} else if (is_loading == 1) {
37293730
PyObject *name = _PyLazyImport_GetName(lazy_import);
@@ -3766,9 +3767,8 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
37663767

37673768
PyObject *globals = PyEval_GetGlobals();
37683769

3769-
PyObject *import_func;
37703770
if (PyMapping_GetOptionalItem(lz->lz_builtins, &_Py_ID(__import__), &import_func) < 0) {
3771-
return NULL;
3771+
goto error;
37723772
}
37733773
if (full) {
37743774
obj = _PyEval_ImportNameWithImport(tstate,
@@ -3781,7 +3781,6 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
37813781
} else {
37823782
PyObject *name = PyUnicode_Substring(lz->lz_from, 0, dot);
37833783
if (name == NULL) {
3784-
Py_DECREF(import_func);
37853784
goto error;
37863785
}
37873786
obj = _PyEval_ImportNameWithImport(tstate,
@@ -3793,7 +3792,6 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
37933792
_PyLong_GetZero());
37943793
Py_DECREF(name);
37953794
}
3796-
Py_DECREF(import_func);
37973795
if (obj == NULL) {
37983796
goto error;
37993797
}
@@ -3891,6 +3889,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
38913889
}
38923890

38933891
Py_XDECREF(fromlist);
3892+
Py_XDECREF(import_func);
38943893
return obj;
38953894
}
38963895

@@ -4226,7 +4225,7 @@ _PyImport_LazyImportModuleLevelObject(PyThreadState *tstate,
42264225
}
42274226

42284227
PyInterpreterState *interp = tstate->interp;
4229-
assert(frame->f_globals == frame->f_locals); // should only be called in global scope
4228+
assert(_PyEval_GetFrame()->f_globals == _PyEval_GetFrame()->f_locals); // should only be called in global scope
42304229

42314230
// Check if the filter disables the lazy import
42324231
PyObject *filter = LAZY_IMPORTS_FILTER(interp);
@@ -5312,7 +5311,7 @@ _imp__set_lazy_attributes_impl(PyObject *module, PyObject *child_module,
53125311
goto error;
53135312
}
53145313

5315-
if (PyDict_SetItem(child_dict, attr_name, lazy_module_attr) < 0) {
5314+
if (_PyModule_ReplaceLazyValue(child_dict, attr_name, lazy_module_attr) < 0) {
53165315
Py_DECREF(lazy_module_attr);
53175316
goto error;
53185317
}

‎Python/specialize.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "pycore_dict.h" // DICT_KEYS_UNICODE
99
#include "pycore_function.h" // _PyFunction_GetVersionForCurrentState()
1010
#include "pycore_interpframe.h" // FRAME_SPECIALS_SIZE
11+
#include "pycore_lazyimportobject.h" // PyLazyImport_CheckExact
1112
#include "pycore_list.h" // _PyListIterObject
1213
#include "pycore_long.h" // _PyLong_IsNonNegativeCompact()
1314
#include "pycore_moduleobject.h"
@@ -541,6 +542,7 @@ _PyCode_Quicken(_Py_CODEUNIT *instructions, Py_ssize_t size, int enable_counters
541542
#define SPEC_FAIL_ATTR_BUILTIN_CLASS_METHOD 22
542543
#define SPEC_FAIL_ATTR_CLASS_METHOD_OBJ 23
543544
#define SPEC_FAIL_ATTR_OBJECT_SLOT 24
545+
#define SPEC_FAIL_ATTR_MODULE_LAZY_VALUE 25
544546

545547
#define SPEC_FAIL_ATTR_INSTANCE_ATTRIBUTE 26
546548
#define SPEC_FAIL_ATTR_METACLASS_ATTRIBUTE 27
@@ -797,6 +799,14 @@ specialize_module_load_attr_lock_held(PyDictObject *dict, _Py_CODEUNIT *instr, P
797799
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OUT_OF_VERSIONS);
798800
return -1;
799801
}
802+
PyObject *value = NULL;
803+
if (PyDict_GetItemRef((PyObject *)dict, name, &value) < 0 ||
804+
(value != NULL && PyLazyImport_CheckExact(value))) {
805+
Py_XDECREF(value);
806+
SPECIALIZATION_FAIL(SPEC_FAIL_ATTR_MODULE_LAZY_VALUE, SPEC_FAIL_OUT_OF_VERSIONS);
807+
return -1;
808+
}
809+
Py_XDECREF(value);
800810
write_u32(cache->version, keys_version);
801811
cache->index = (uint16_t)index;
802812
specialize(instr, LOAD_ATTR_MODULE);

0 commit comments

Comments
 (0)