gh-149728: Fix free-threaded race in importlib lazy-submodule fast path

[SwayamInSync]

Fixes #149728.

Cause

_load_unlocked clears spec._initializing at the end of its body, before _find_and_load_unlocked runs setattr(parent_module, child, module). Between those two events, sys.modules[name] is set and _initializing == False, but parent.__dict__[child] is still missing. The fast path in _find_and_load returns the module without taking the import lock once it sees _initializing == False, so under free-threaded CPython a second thread can observe this window. IMPORT_FROM 'child' on that thread does getattr(parent, 'child'), falls into a lazy __getattr__, runs the same import parent.child as child line, fast-paths again, and recurses to RecursionError. See the linked issue for the full walkthrough and reproducer.

Change

Keep spec._initializing == True until after the parent setattr in _find_and_load_unlocked:

• _load_unlocked no longer clears _initializing on the success path. Failure paths still clear it.
• _find_and_load_unlocked clears _initializing in a finally block after setattr(parent_module, child, module) and _imp._set_lazy_attributes.
• The two other callers of _load_unlocked (_load and _builtin_from_name) have no parent setattr, so they clear _initializing in a local finally.

This preserves the invariant the fast path needs: _initializing == False implies the module is reachable via getattr(parent_module, child).

Test

Lib/test/test_importlib/test_threaded_import.py::ThreadedImportTests::test_lazy_submodule_getattr_no_recursion widens the natural microsecond race window with one threading.Event and verifies that an observer thread does not recurse on the lazy __getattr__. Fails on the unpatched interpreter, passes on this branch. Full test_importlib suite remains green (1220/1220).

Notes

• Tested on macOS arm64 with both the free-threaded build (3.16.0a0 from this branch) and the system GIL build (3.14.3). The bug reproduces on both builds under the deterministic shim, confirming the code path itself is build-independent; free-threading just makes the window naturally observable.

• Issue: Free-threaded importlib race recurses on lazy-submodule __getattr__ #149728

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[kumaraditya303]

no need to say the internal details, something like "fix race in importlib when importing module which defined getattr" should be enough

[SwayamInSync]

Done

[DinoV]

I may not be the best person to review this, but sprinkling the spec._initializing = False calls through out seems a little fragile. I would suggest doing something like adding a finish_load = None parameter to _load_unlocked and if it's not None then you call it here after all of the the other initialization has succeeded.

Most of the call sites then don't change, but _find_and_load_unlocked and then move the parent module initialization into a nested function and pass that in.

[brettcannon]

A quick glance gives me the same concern around the sprinkling.

[brettcannon]

The approach seems fragile.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[SwayamInSync]

Reworked along the lines @DinoV suggested.
_load_unlocked() now takes an optional finish_load callback that runs just before spec._initializing is cleared, and _find_and_load_unlocked() does the parent setattr() and _imp._set_lazy_attributes() from it. _load() and _builtin_from_name() are untouched again. The flag is set and cleared in exactly one place, instead of the five clear sites the previous version scattered around.

The test no longer monkeypatches _bootstrap. It hooks the test package's own __setattr__ to hold the binding window open, so the real _load_unlocked() and _find_and_load_unlocked() run unmodified.
Its main assertion, that the submodule's spec is still _initializing when it is bound on its parent, is independent of thread scheduling.