Skip to content

Commit 6f16938

Browse files
committed
More thread safety fixes
1 parent b9131b5 commit 6f16938

File tree

2 files changed

+51
-24
lines changed

2 files changed

+51
-24
lines changed

Include/internal/pycore_interp_structs.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,9 @@ struct _import_state {
327327
PyObject *lazy_imports_filter;
328328
PyObject *lazy_importing_modules;
329329
PyObject *lazy_modules;
330+
#ifdef Py_GIL_DISABLED
331+
PyMutex lazy_mutex;
332+
#endif
330333
/* The global import lock. */
331334
_PyRecursiveMutex lock;
332335
/* diagnostic info in PyImport_ImportModuleLevelObject() */

Python/import.c

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ static struct _inittab *inittab_copy = NULL;
122122
#define LAZY_IMPORTS_FILTER(interp) \
123123
(interp)->imports.lazy_imports_filter
124124

125+
#ifdef Py_GIL_DISABLED
126+
#define LAZY_IMPORTS_LOCK(interp) PyMutex_Lock(&(interp)->imports.lazy_mutex)
127+
#define LAZY_IMPORTS_UNLOCK(interp) PyMutex_Unlock(&(interp)->imports.lazy_mutex)
128+
#else
129+
#define LAZY_IMPORTS_LOCK(interp)
130+
#define LAZY_IMPORTS_UNLOCK(interp)
131+
#endif
132+
133+
125134
#define _IMPORT_TIME_HEADER(interp) \
126135
do { \
127136
if (FIND_AND_LOAD((interp)).header) { \
@@ -4248,6 +4257,26 @@ get_mod_dict(PyObject *module)
42484257
return PyObject_GetAttr(module, &_Py_ID(__dict__));
42494258
}
42504259

4260+
// ensure we have the set for the parent module name in sys.lazy_modules.
4261+
// Returns a new reference.
4262+
static PyObject *
4263+
ensure_lazy_submodules(PyDictObject *lazy_modules, PyObject *parent)
4264+
{
4265+
PyObject *lazy_submodules;
4266+
Py_BEGIN_CRITICAL_SECTION(lazy_modules);
4267+
int err = _PyDict_GetItemRef_Unicode_LockHeld(lazy_modules, parent, &lazy_submodules);
4268+
if (err == 0) {
4269+
// value isn't present
4270+
lazy_submodules = PySet_New(NULL);
4271+
if (lazy_submodules != NULL &&
4272+
_PyDict_SetItem_LockHeld(lazy_modules, parent, lazy_submodules) < 0) {
4273+
Py_CLEAR(lazy_submodules);
4274+
}
4275+
}
4276+
Py_END_CRITICAL_SECTION();
4277+
return lazy_submodules;
4278+
}
4279+
42514280
static int
42524281
register_lazy_on_parent(PyThreadState *tstate, PyObject *name, PyObject *builtins)
42534282
{
@@ -4282,20 +4311,12 @@ register_lazy_on_parent(PyThreadState *tstate, PyObject *name, PyObject *builtin
42824311
}
42834312

42844313
// Record the child as being lazily imported from the parent.
4285-
PyObject *lazy_submodules;
4286-
if (PyDict_GetItemRef(lazy_modules, parent, &lazy_submodules) < 0) {
4287-
goto done;
4288-
}
4314+
PyObject *lazy_submodules = ensure_lazy_submodules((PyDictObject *)lazy_modules,
4315+
parent);
42894316
if (lazy_submodules == NULL) {
4290-
lazy_submodules = PySet_New(NULL);
4291-
if (lazy_submodules == NULL) {
4292-
goto done;
4293-
}
4294-
if (PyDict_SetItem(lazy_modules, parent, lazy_submodules) < 0) {
4295-
Py_DECREF(lazy_submodules);
4296-
goto done;
4297-
}
4317+
goto done;
42984318
}
4319+
42994320
if (PySet_Add(lazy_submodules, child) < 0) {
43004321
Py_DECREF(lazy_submodules);
43014322
goto done;
@@ -4379,8 +4400,10 @@ _PyImport_LazyImportModuleLevelObject(PyThreadState *tstate,
43794400
// Check if the filter disables the lazy import.
43804401
// We must hold a reference to the filter while calling it to prevent
43814402
// use-after-free if another thread replaces it via PyImport_SetLazyImportsFilter.
4382-
PyObject *filter = FT_ATOMIC_LOAD_PTR_RELAXED(LAZY_IMPORTS_FILTER(interp));
4383-
Py_XINCREF(filter);
4403+
LAZY_IMPORTS_LOCK(interp);
4404+
PyObject *filter = Py_XNewRef(LAZY_IMPORTS_FILTER(interp));
4405+
LAZY_IMPORTS_UNLOCK(interp);
4406+
43844407
if (filter != NULL) {
43854408
PyObject *modname;
43864409
if (PyDict_GetItemRef(globals, &_Py_ID(__name__), &modname) < 0) {
@@ -4809,15 +4832,13 @@ PyImport_SetLazyImportsFilter(PyObject *filter)
48094832
}
48104833

48114834
PyInterpreterState *interp = _PyInterpreterState_GET();
4812-
#ifdef Py_GIL_DISABLED
4813-
// Exchange the filter atomically. Use deferred DECREF to prevent
4814-
// use-after-free: another thread may have loaded the old filter
4815-
// and be about to INCREF it.
4816-
PyObject *old = _Py_atomic_exchange_ptr(&LAZY_IMPORTS_FILTER(interp), Py_XNewRef(filter));
4817-
_PyObject_XDecRefDelayed(old);
4818-
#else
4819-
Py_XSETREF(LAZY_IMPORTS_FILTER(interp), Py_XNewRef(filter));
4820-
#endif
4835+
// Exchange the filter w/ the lock held. We can't use Py_XSETREF
4836+
// because we need to release the lock before the decref.
4837+
LAZY_IMPORTS_LOCK(interp);
4838+
PyObject *old = LAZY_IMPORTS_FILTER(interp);
4839+
LAZY_IMPORTS_FILTER(interp) = Py_XNewRef(filter);
4840+
LAZY_IMPORTS_UNLOCK(interp);
4841+
Py_XDECREF(old);
48214842
return 0;
48224843
}
48234844

@@ -4828,7 +4849,10 @@ PyObject *
48284849
PyImport_GetLazyImportsFilter(void)
48294850
{
48304851
PyInterpreterState *interp = _PyInterpreterState_GET();
4831-
return Py_XNewRef(FT_ATOMIC_LOAD_PTR_RELAXED(LAZY_IMPORTS_FILTER(interp)));
4852+
LAZY_IMPORTS_LOCK(interp);
4853+
PyObject *res = Py_XNewRef(LAZY_IMPORTS_FILTER(interp));
4854+
LAZY_IMPORTS_UNLOCK(interp);
4855+
return res;
48324856
}
48334857

48344858
int

0 commit comments

Comments
 (0)