Skip to content

Commit 83b692b

Browse files
brittanyreybrittanyrey
committed
Fix free-threaded race reifying a lazy submodule attribute
A lazily imported submodule is tracked in two places: the binding on its parent package, and the parent's entry in the interpreter's pending-submodule map. Reading an attribute consults both, and nothing ordered the two updates against each other, so in the free-threaded build: 1. Thread A looks up pkg.sub, misses it in pkg.__dict__, and falls through to the lazy-submodule path. 2. Thread B gets there first: it imports pkg.sub, binds it on pkg, and removes 'sub' from pkg's pending set. 3. Thread A checks that set, finds the name gone, concludes 'sub' was never a lazy submodule, and raises AttributeError for an attribute that by then exists. Make the parent's __dict__ authoritative and the pending set a hint that can only go stale in one direction: _PyImport_TryLoadLazySubmodule() stores the binding before dropping the name from the pending set, and re-reads the parent's __dict__ when it finds the name already gone. A thread that observes the removal is then guaranteed to see the binding. Issue: pythongh-154561
1 parent e2ff498 commit 83b692b

5 files changed

Lines changed: 64 additions & 11 deletions

File tree

‎Include/internal/pycore_import.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ typedef enum {
4545
_Py_LAZY_SUBMODULE_LOADED = 1,
4646
} _PyLazySubmoduleImportResult;
4747
extern _PyLazySubmoduleImportResult _PyImport_TryLoadLazySubmodule(
48-
PyObject *mod_name, PyObject *attr_name, PyObject **result);
48+
PyObject *parent, PyObject *mod_name, PyObject *attr_name,
49+
PyObject **result);
4950
extern PyObject * _PyImport_LazyImportModuleLevelObject(
5051
PyThreadState *tstate, PyObject *name, PyObject *builtins,
5152
PyObject *globals, PyObject *locals, PyObject *fromlist, int level);

‎Lib/test/test_lazy_import/__init__.py‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import contextlib
1414

1515
from test import support
16+
from test.support import threading_helper
1617
from test.support.script_helper import assert_python_ok
1718

1819
try:
@@ -26,12 +27,15 @@ def setUp(self):
2627
self.lazy_imports_filter = sys.get_lazy_imports_filter()
2728
self.lazy_imports = sys.get_lazy_imports()
2829

29-
def tearDown(self):
30-
"""Clean up any test modules from sys.modules."""
30+
def unload_data_modules(self):
31+
"""Remove the test data package and its submodules from sys.modules."""
3132
for key in list(sys.modules.keys()):
3233
if key.startswith('test.test_lazy_import.data'):
3334
del sys.modules[key]
3435

36+
def tearDown(self):
37+
"""Clean up any test modules from sys.modules."""
38+
self.unload_data_modules()
3539
sys.set_lazy_imports_filter(self.lazy_imports_filter)
3640
sys.set_lazy_imports(self.lazy_imports)
3741
sys.lazy_modules.clear()
@@ -1876,6 +1880,27 @@ def access_lazy_import(idx):
18761880
for r in results[1:]:
18771881
self.assertIs(r, first_module)
18781882

1883+
def test_concurrent_lazy_submodule_attribute(self):
1884+
"""Threads racing on a pending lazy submodule attribute all get it."""
1885+
pkg = 'test.test_lazy_import.data'
1886+
num_threads = 4
1887+
for _ in range(10):
1888+
self.unload_data_modules()
1889+
# Registers 'basic2' as a lazy submodule of 'data'.
1890+
import test.test_lazy_import.data.basic_unused
1891+
data = sys.modules[pkg]
1892+
# Leave 'basic2' in sys.modules but unbound on 'data', so every
1893+
# thread misses it in data.__dict__ and races to reify it.
1894+
import test.test_lazy_import.data.basic2
1895+
del data.basic2
1896+
1897+
results = []
1898+
threading_helper.run_concurrently(
1899+
lambda: results.append(data.basic2), num_threads)
1900+
1901+
basic2 = sys.modules[f'{pkg}.basic2']
1902+
self.assertEqual(results, [basic2] * num_threads)
1903+
18791904
def test_concurrent_reification_multiple_modules(self):
18801905
"""Multiple threads reifying different lazy imports concurrently."""
18811906
code = textwrap.dedent("""

‎Misc/NEWS.d/next/Core_and_Builtins/2026-09-17-19-05-12.gh-issue-154561.CmydWb.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a race condition in the free-threaded build where threads concurrently
2+
accessing the same lazily imported submodule attribute of a package could
3+
raise :exc:`AttributeError` instead of the submodule.

‎Objects/moduleobject.c‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,17 +1313,15 @@ try_load_lazy_submodule(PyModuleObject *m, PyObject *name)
13131313
}
13141314
PyObject *result = NULL;
13151315
_PyLazySubmoduleImportResult status =
1316-
_PyImport_TryLoadLazySubmodule(mod_name, name, &result);
1316+
_PyImport_TryLoadLazySubmodule((PyObject *)m, mod_name, name, &result);
13171317
Py_DECREF(mod_name);
13181318
if (status != _Py_LAZY_SUBMODULE_LOADED) {
13191319
assert(status == _Py_LAZY_SUBMODULE_ERROR ||
13201320
status == _Py_LAZY_SUBMODULE_NOT_FOUND);
1321+
assert(result == NULL);
13211322
return NULL;
13221323
}
1323-
if (PyDict_SetItem(m->md_dict, name, result) < 0) {
1324-
Py_DECREF(result);
1325-
return NULL;
1326-
}
1324+
// _PyImport_TryLoadLazySubmodule() has bound the submodule in md_dict.
13271325
return result;
13281326
}
13291327

‎Python/import.c‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4443,15 +4443,21 @@ register_from_lazy_on_parent(PyThreadState *tstate, PyObject *abs_name,
44434443
return res;
44444444
}
44454445

4446+
// Reify the lazy submodule 'mod_name.attr_name' and bind it on the module
4447+
// 'parent'. The binding is stored before 'attr_name' is dropped from the set
4448+
// of pending submodules, and names are only ever added to 'lazy_pending', so a
4449+
// thread that misses the attribute in the module dict and then finds no
4450+
// pending entry can tell that another thread has just bound it.
44464451
_PyLazySubmoduleImportResult
4447-
_PyImport_TryLoadLazySubmodule(PyObject *mod_name, PyObject *attr_name,
4448-
PyObject **result)
4452+
_PyImport_TryLoadLazySubmodule(PyObject *parent, PyObject *mod_name,
4453+
PyObject *attr_name, PyObject **result)
44494454
{
44504455
assert(result != NULL);
44514456
*result = NULL;
44524457

44534458
PyThreadState *tstate = _PyThreadState_GET();
44544459
PyInterpreterState *interp = tstate->interp;
4460+
PyObject *parent_dict = _PyModule_GetDict(parent);
44554461
PyObject *lazy_pending = LAZY_PENDING_SUBMODULES(interp);
44564462
if (lazy_pending == NULL) {
44574463
return _Py_LAZY_SUBMODULE_NOT_FOUND;
@@ -4473,7 +4479,21 @@ _PyImport_TryLoadLazySubmodule(PyObject *mod_name, PyObject *attr_name,
44734479
}
44744480
if (contains == 0) {
44754481
Py_DECREF(pending_set);
4476-
return _Py_LAZY_SUBMODULE_NOT_FOUND;
4482+
// Another thread may have reified 'attr_name' and dropped it from
4483+
// 'pending_set' since our caller looked in the module dict.
4484+
rc = PyDict_GetItemRef(parent_dict, attr_name, result);
4485+
if (rc < 0) {
4486+
return _Py_LAZY_SUBMODULE_ERROR;
4487+
}
4488+
if (rc == 0) {
4489+
return _Py_LAZY_SUBMODULE_NOT_FOUND;
4490+
}
4491+
if (PyLazyImport_CheckExact(*result)) {
4492+
// 'parent' is binding its own lazy import of this name.
4493+
Py_CLEAR(*result);
4494+
return _Py_LAZY_SUBMODULE_NOT_FOUND;
4495+
}
4496+
return _Py_LAZY_SUBMODULE_LOADED;
44774497
}
44784498

44794499
PyObject *full_name = PyUnicode_FromFormat("%U.%U", mod_name, attr_name);
@@ -4497,6 +4517,12 @@ _PyImport_TryLoadLazySubmodule(PyObject *mod_name, PyObject *attr_name,
44974517
return _Py_LAZY_SUBMODULE_NOT_FOUND;
44984518
}
44994519

4520+
if (PyDict_SetItem(parent_dict, attr_name, mod) < 0) {
4521+
Py_DECREF(mod);
4522+
Py_DECREF(pending_set);
4523+
Py_DECREF(full_name);
4524+
return _Py_LAZY_SUBMODULE_ERROR;
4525+
}
45004526
if (PySet_Discard(pending_set, attr_name) < 0) {
45014527
Py_DECREF(mod);
45024528
Py_DECREF(pending_set);

0 commit comments

Comments
 (0)