Skip to content

Commit 239c717

Browse files
committed
gh-142349: Fix refcount corruption in lazy import specialization
The LOAD_GLOBAL specialization path in specialize_load_global_lock_held() called Py_DECREF(value) on the result of _PyDict_LookupIndexAndValue(), which returns a borrowed reference via _Py_dict_lookup(). This spurious DECREF silently corrupted the reference count of lazy import objects each time the adaptive specialization counter triggered while the lazy import was still present in the globals dict. This happens during concurrent lazy import reification with 8 or more threads in practice. The reason the thread count mattered is that with more threads racing through the barrier, more of them got a turn on the GIL to execute the LOAD_GLOBAL instruction (and trigger its adaptive specialization counter) while the first thread was still inside the import machinery resolving the lazy import. Each such trigger stole one reference from the lazy import object. With 2-7 threads, typically only 1-2 specialization triggers fired before the import completed, so the object survived. With 8+ threads, enough triggers accumulated (3 or more) to drive the reference count to zero while other threads still held pointers to the object, causing use-after-free. The fix removes the erroneous Py_DECREF from the specialization guard. The module attribute specialization path (specialize_module_load_attr) correctly handled the same case without a DECREF and served as confirmation that the LOAD_GLOBAL path was wrong. This commit also adds the lz_resolved field to PyLazyImportObject, which caches the resolved module after the first thread completes the import. Subsequent threads that were blocked on the import lock can then return the cached result immediately rather than re-importing. The _PyImport_LoadLazyImportTstate function now takes a strong reference to the lazy import at entry (to keep it alive across the GIL release inside _PyImport_AcquireLock) and releases it at every exit path, and a double-release of the import lock in the PySet_Add error path has also been corrected.
1 parent 46d5106 commit 239c717

File tree

4 files changed

+37
-2
lines changed

4 files changed

+37
-2
lines changed

Include/internal/pycore_lazyimportobject.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ typedef struct {
2222
// Frame information for the original import location.
2323
PyCodeObject *lz_code; // Code object where the lazy import was created.
2424
int lz_instr_offset; // Instruction offset where the lazy import was created.
25+
// Cached result from the first thread to resolve this lazy import.
26+
// Protected by the global import lock. When non-NULL, subsequent
27+
// threads skip the import and return this value directly.
28+
PyObject *lz_resolved;
2529
} PyLazyImportObject;
2630

2731

Objects/lazyimportobject.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ _PyLazyImport_New(_PyInterpreterFrame *frame, PyObject *builtins, PyObject *name
3737
// Capture frame information for the original import location.
3838
m->lz_code = NULL;
3939
m->lz_instr_offset = -1;
40+
m->lz_resolved = NULL;
4041

4142
if (frame != NULL) {
4243
PyCodeObject *code = _PyFrame_GetCode(frame);
@@ -59,6 +60,7 @@ lazy_import_traverse(PyObject *op, visitproc visit, void *arg)
5960
Py_VISIT(m->lz_from);
6061
Py_VISIT(m->lz_attr);
6162
Py_VISIT(m->lz_code);
63+
Py_VISIT(m->lz_resolved);
6264
return 0;
6365
}
6466

@@ -70,6 +72,7 @@ lazy_import_clear(PyObject *op)
7072
Py_CLEAR(m->lz_from);
7173
Py_CLEAR(m->lz_attr);
7274
Py_CLEAR(m->lz_code);
75+
Py_CLEAR(m->lz_resolved);
7376
return 0;
7477
}
7578

Python/import.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* Module definition and import implementation */
22

33
#include "Python.h"
4+
45
#include "pycore_audit.h" // _PySys_Audit()
56
#include "pycore_ceval.h"
67
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
@@ -3881,12 +3882,29 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
38813882
assert(lazy_import != NULL);
38823883
assert(PyLazyImport_CheckExact(lazy_import));
38833884

3885+
// Keep a strong reference to the lazy import object. During
3886+
// _PyImport_AcquireLock() below the GIL may be released, allowing
3887+
// another thread to resolve this same lazy import and drop all
3888+
// external references (from the globals dict and stack). The extra
3889+
// INCREF ensures the object stays alive while we work with it.
3890+
Py_INCREF(lazy_import);
3891+
38843892
PyLazyImportObject *lz = (PyLazyImportObject *)lazy_import;
38853893
PyInterpreterState *interp = tstate->interp;
38863894

38873895
// Acquire the global import lock to serialize reification
38883896
_PyImport_AcquireLock(interp);
38893897

3898+
// Check if another thread already resolved this lazy import while
3899+
// we were waiting for the lock. If so, return the cached result.
3900+
PyObject *resolved = FT_ATOMIC_LOAD_PTR_ACQUIRE(lz->lz_resolved);
3901+
if (resolved != NULL) {
3902+
obj = Py_NewRef(resolved);
3903+
_PyImport_ReleaseLock(interp);
3904+
Py_DECREF(lazy_import);
3905+
return obj;
3906+
}
3907+
38903908
// Check if we are already importing this module, if so, then we want to
38913909
// return an error that indicates we've hit a cycle which will indicate
38923910
// the value isn't yet available.
@@ -3895,20 +3913,24 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
38953913
importing = interp->imports.lazy_importing_modules = PySet_New(NULL);
38963914
if (importing == NULL) {
38973915
_PyImport_ReleaseLock(interp);
3916+
Py_DECREF(lazy_import);
38983917
return NULL;
38993918
}
39003919
}
39013920

39023921
assert(PyAnySet_CheckExact(importing));
3922+
39033923
int is_loading = _PySet_Contains((PySetObject *)importing, lazy_import);
39043924
if (is_loading < 0) {
39053925
_PyImport_ReleaseLock(interp);
3926+
Py_DECREF(lazy_import);
39063927
return NULL;
39073928
}
39083929
else if (is_loading == 1) {
39093930
PyObject *name = _PyLazyImport_GetName(lazy_import);
39103931
if (name == NULL) {
39113932
_PyImport_ReleaseLock(interp);
3933+
Py_DECREF(lazy_import);
39123934
return NULL;
39133935
}
39143936
PyObject *errmsg = PyUnicode_FromFormat(
@@ -3917,17 +3939,18 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
39173939
if (errmsg == NULL) {
39183940
Py_DECREF(name);
39193941
_PyImport_ReleaseLock(interp);
3942+
Py_DECREF(lazy_import);
39203943
return NULL;
39213944
}
39223945
PyErr_SetImportErrorSubclass(PyExc_ImportCycleError, errmsg,
39233946
lz->lz_from, NULL);
39243947
Py_DECREF(errmsg);
39253948
Py_DECREF(name);
39263949
_PyImport_ReleaseLock(interp);
3950+
Py_DECREF(lazy_import);
39273951
return NULL;
39283952
}
39293953
else if (PySet_Add(importing, lazy_import) < 0) {
3930-
_PyImport_ReleaseLock(interp);
39313954
goto error;
39323955
}
39333956

@@ -3986,6 +4009,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
39864009
);
39874010
Py_DECREF(name);
39884011
}
4012+
39894013
if (obj == NULL) {
39904014
goto error;
39914015
}
@@ -4001,6 +4025,10 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
40014025

40024026
assert(!PyLazyImport_CheckExact(obj));
40034027

4028+
// Cache the resolved result so other threads waiting on the import
4029+
// lock can return it immediately without re-importing.
4030+
FT_ATOMIC_STORE_PTR_RELEASE(lz->lz_resolved, Py_NewRef(obj));
4031+
40044032
goto ok;
40054033

40064034
error:
@@ -4090,6 +4118,7 @@ _PyImport_LoadLazyImportTstate(PyThreadState *tstate, PyObject *lazy_import)
40904118

40914119
Py_XDECREF(fromlist);
40924120
Py_XDECREF(import_func);
4121+
Py_DECREF(lazy_import);
40934122
return obj;
40944123
}
40954124

Python/specialize.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,6 @@ specialize_load_global_lock_held(
13211321
}
13221322
if (value != NULL && PyLazyImport_CheckExact(value)) {
13231323
SPECIALIZATION_FAIL(LOAD_GLOBAL, SPEC_FAIL_ATTR_MODULE_LAZY_VALUE);
1324-
Py_DECREF(value);
13251324
goto fail;
13261325
}
13271326
PyInterpreterState *interp = _PyInterpreterState_GET();

0 commit comments

Comments
 (0)