Keep module cache entries present during import reordering by youknowdot · Pull Request #8948 · RustPython/RustPython · GitHub

Summary

Keep exact dict entries in sys.modules present while importlib updates shutdown order. The old pop() / assignment pair exposes a temporary absence to other importing threads. This is independently based on current Python 3.14-targeting main; it does not switch the target or the cached importer callback.

Add a private _imp._dict_move_to_end operation for exact dictionaries, with lookup callbacks outside the storage guard and relocation under one write guard
Preserve the current value, stored key and hash; invalidate layout caches and retain iterator/compaction bookkeeping
Revalidate both matched candidates and previously probed prefixes after reentrant equality callbacks, including nested deletion/compaction and insertion into a visited dummy bucket
Use the helper at all four current importlib reorder sites; retain the original pop / assignment behavior for custom mappings, dict subclasses and CPython
Add deterministic frozen/source bootstrap regressions and native cache/iterator/compaction tests

Semantic scope

This is a true move rather than a universally equivalent pop / assignment pair. It hashes once, preserves the stored key object, pins one dictionary and lookup key, and makes an already-last entry a no-op. Exact-dict callback side effects that depended on a second hash, key replacement, or rebinding sys.modules between the two Python operations intentionally differ. The custom-mapping fallback preserves those original Python operations and is not atomic.

Deliberate module removal/replacement and whole-map rebinding remain possible; this eliminates the artificial gap caused by shutdown-order maintenance. Identity hits in the first bucket allocate no probe-prefix storage. Pathological reentrant colliding keys can require O(k²) prefix validation and O(k) retained references.

This is a prerequisite for the separately prepared Python 3.15 bootstrap and canonical importer callback migration. It does not claim to fix the pre-existing Python 3.14 hierarchical-import deadlock.

Validation

The standalone release was built from this branch, and its frozen/source bootstrap, private helper, target 3.14 and source-Lib provenance were checked.

Native regression snippet and the independently identified false-equality/prefix regression: pass
test_importlib test_import test_site test_support test_pkgutil test_dict test_ordered_dict: 2,096 tests run, 131 skipped, no failures
Full bounded Python snippets: 469 passed, 13 failures; 10 AF_UNIX sandbox failures, two chown errno differences, and one bounded CPython GC/import stress timeout. The new native regression passes; this is not a green full-snippet claim
Fresh workspace Rust tests: 1,348 passed, 18 ignored, including all three new dict_inner::tests::move_to_end_* tests
Fresh separate C-API tests: 115 passed, 4 ignored
Workspace/C-API Clippy: both passed without warnings
Final regression snippet: native RustPython, CPython 3.14.7 and CPython 3.15.0rc2 all pass. Legacy cases remain enabled whenever the bootstrap exposes them; modern load/exec cases always run

Rust/Lib implementation and aggregate checks are at 2e30cbf. The final head only adds the independently rerun snippet availability guard; production source is byte-identical.

A shared-target stale VM test binary was detected because it lacked the new tests. That first aggregate run is discarded as validation. The reported fresh runs rebuilt the affected sources, verified the new test names, and checked the resulting binary provenance. No source content was changed to repair that cache issue.

AI assistance

Implemented, reviewed and tested with OpenAI Codex under the requesting RustPython maintainer's direction. No human code review or maintainer privilege is implied for this account. The runtime does not expose an exact model version; commits use Assisted-by: Codex:model-version-unavailable rather than inventing one.

Summary by CodeRabbit

New Features
- Dictionaries now support moving an existing key-value entry to the end of insertion order. The operation returns the value and raises a KeyError if the key is missing.
Bug Fixes
- Dictionary ordering remains consistent when entries are moved, including repeated moves and changes made during the operation.
- Iterators are invalidated when a relocation changes dictionary order.

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used 📚 Code guidelines (1)

AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: 4c006ebe-f905-4488-9ea4-39281d37bd0c

📥 Commits

Reviewing files that changed from the base of the PR and between ee8a4e7 and ddace94.

📒 Files selected for processing (2)

crates/vm/src/stdlib/_imp.rs
extra_tests/snippets/import_atomic_reorder.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The change adds Dict::move_to_end, exposes it through the exact-dictionary API and _imp, and adds tests for ordering, lookup behavior, callback-driven mutations, and iterator invalidation.

Changes

Dictionary Reordering

Layer / File(s)	Summary
Dictionary move operation crates/vm/src/dict_inner.rs, crates/vm/src/builtins/dict.rs, crates/vm/src/stdlib/_imp.rs	Dict::move_to_end moves a matching entry to the end and returns its value, or returns None when absent. The exact-dictionary API raises KeyError for a missing key. The _imp._dict_move_to_end helper exposes the operation.
Move behavior validation crates/vm/src/dict_inner.rs, extra_tests/snippets/import_atomic_reorder.py	Tests check entry order, cache and iterator invalidation, compaction, and storage consistency. The Python snippet also tests lookup errors, callback-driven mutations, finalizers, and iterator invalidation.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ddace

This adds a private helper for reordering dictionary entries without briefly removing them from the module cache. No concrete merge-blocking risk was found in the supplied changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ddace

The new helper operates only on a dictionary supplied by its caller and preserves entries during relocation. No new privilege or isolation bypass was demonstrated. However, the production import-ordering code remains unchanged, and concurrent runtime behavior was not validated in this review.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The independently mutable scope is the exact dictionary supplied by the caller. Supplying sys.modules would affect interpreter-wide module-cache ordering, but the helper does not obtain additional dictionaries, credentials, or privileged resources on the caller's behalf. Its exposure requires Python-level access to the helper and the dictionary object.

Trust Boundaries and Controls

inferred — Caller-controlled keys can execute hash and equality callbacks, as with existing dictionary operations. The new operation introduces no demonstrated authority escalation: it mutates an already-referenced mutable dictionary. Exact-type enforcement avoids subclass mapping overrides, while unlocked callbacks and witness revalidation protect storage consistency rather than acting as authorization controls.

Resilience and Maintainability Implications

observed — Callback exceptions occur before relocation, and candidate and probe-reference cleanup occurs outside the storage guard. Regressions exercise replacement, clearing, resizing, nested relocation, insertion into a visited dummy bucket, exception propagation, and finalizer-driven mutation. These checks support failure containment for reentrant callbacks, but do not establish runtime behavior under actual concurrent threads.

Hardening Proposals

proposed — Before integrating this primitive into module-cache maintenance, validate concurrent readers and mutators against the continuous-presence invariant, and verify each reorder site in both source and frozen bootstrap configurations. This is validation guidance, not an observed vulnerability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.13% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main objective: keep module cache entries present while import ordering changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] lib: cpython/Lib/importlib
[ ] test: cpython/Lib/test/test_importlib (TODO: 8)

dependencies:

importlib (native: _abc, _adapters, _bootstrap, _bootstrap_external, _collections, _common, _frozen_importlib, _frozen_importlib_external, _functional, _functools, _imp, _io, _itertools, _meta, _text, _warnings, collections.abc, email.message, importlib.abc, importlib.metadata, importlib.readers, itertools, marshal, nt, posix, resources.readers, resources.simple, sys, winreg)
- io (native: _io, _thread, errno, msvcrt, sys)
- json (native: _json, decoder, encoder, json.tool, sys)
- warnings (native: _contextvars, _thread, _warnings, builtins, sys)
- future, abc, collections, contextlib, csv, email, functools, inspect, operator, os, pathlib, posixpath, re, tempfile, textwrap, threading, tokenize, types, typing, zipfile

dependent tests: (129 tests)

importlib: test_asdl_parser test_bdb test_cmd_line_script test_codecs test_compileall test_ctypes test_doctest test_external_inspection test_frozen test_hashlib test_importlib test_inspect test_linecache test_modulefinder test_multiprocessing_main_handling test_pkgutil test_py_compile test_pyclbr test_pydoc test_pyrepl test_reprlib test_runpy test_sundry test_support test_tomllib test_tools test_unittest test_zipfile test_zipimport test_zoneinfo
- ctypes.util: test_ctypes
- ensurepip: test_ensurepip test_venv
- idlelib: test_idle
- inspect: test_abc test_argparse test_asyncgen test_buffer test_builtin test_clinic test_code test_collections test_coroutines test_decimal test_enum test_functools test_generators test_grammar test_monitoring test_ntpath test_operator test_patma test_posixpath test_signal test_sqlite3 test_traceback test_turtle test_type_annotations test_type_params test_types test_typing test_unittest test_yield_from test_zipimport_support
  - ast: test_ast test_codeop test_compile test_compiler_codegen test_dis test_fstring test_future_stmt test_peepholer test_peg_generator test_site test_ssl test_type_comments test_ucn test_unparse
  - asyncio: test_asyncio test_concurrent_futures test_logging test_os test_pdb test_unittest
  - cmd: test_cmd
  - dataclasses: test__colorize test_copy test_ctypes test_genericalias test_pprint test_regrtest
  - rlcompleter: test_pyrepl test_rlcompleter
  - trace: test_trace
  - xmlrpc.server: test_docxmlrpc test_xmlrpc
- profile: test_profile
- py_compile: test_importlib
  - zipfile: test_shutil test_zipapp test_zipfile test_zipfile64
- sysconfig: test_c_locale_coercion test_cmd_line test_dtrace test_embed test_gc test_launcher test_osx_env test_peg_generator test_posix test_pyexpat test_subprocess test_sys test_sysconfig test_time test_tools test_urllib2net
- zipfile:
  - shutil: test_bz2 test_filecmp test_glob test_httpservers test_largefile test_sax test_string_literals test_tarfile test_tempfile test_unicode_file
- zipimport: test_importlib

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Add a private exact-dict move primitive and importlib dispatcher so shutdown-order updates never temporarily remove a module. Preserve current values and custom-mapping fallback behavior; validate reentrant equality probe witnesses, invalidate layout caches, and compact moved entries. Cover all four existing bootstrap reorder sites, including legacy-loader cleanup. Add deterministic trace-window, loader replacement/removal, reentrancy, cache, iterator and compaction regressions. The private native primitive uses true-move semantics; it does not promise arbitrary pop/set callback equivalence. Assisted-by: Codex:model-version-unavailable

Run legacy-loader cases when the bootstrap exposes that helper, preserving all four 3.14 paths and the two remaining 3.15 paths. Always retain modern load and exec regressions. Assisted-by: Codex:model-version-unavailable

Restore the exact CPython 3.14 bootstrap and retain only the private exact-dict relocation primitive and its direct native regressions. Remove the PR-added callback integration tests while preserving native helper assertions. This prepares a primitive only; imports do not call it and the import race remains unresolved. Assisted-by: Codex:model-version-unavailable

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing youknowdot:cpython-import-atomic-reorder (ddace94) with main (04d9990)

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

fanninpm requested a review from youknowone October 2, 2026 17:08

This was referenced Oct 2, 2026

Begin the Python 3.15 target and essential-library migration #8950

Draft

Prepare Python 3.15 with rc3 libraries and native prerequisites #8954

Draft

youknowone marked this pull request as ready for review October 5, 2026 00:04

youknowone reviewed Oct 5, 2026

View reviewed changes

Comment thread Lib/importlib/_bootstrap.py

youknowdot added 3 commits October 5, 2026 17:49

Keep atomic import regression compatible with modern bootstrap

5dab876

Run legacy-loader cases when the bootstrap exposes that helper, preserving all four 3.14 paths and the two remaining 3.15 paths. Always retain modern load and exec regressions. Assisted-by: Codex:model-version-unavailable

youknowdot force-pushed the cpython-import-atomic-reorder branch from ee8a4e7 to ddace94 Compare October 5, 2026 08:51

youknowone approved these changes Oct 5, 2026

View reviewed changes

youknowone merged commit d3ff1eb into RustPython:main Oct 5, 2026
30 checks passed

Keep module cache entries present during import reordering by youknowdot · Pull Request #8948 · RustPython/RustPython · GitHub

Repository navigation

Keep module cache entries present during import reordering - #8948

Keep module cache entries present during import reordering#8948
youknowone merged 3 commits into
RustPython:mainfrom
youknowdot:cpython-import-atomic-reorder

youknowdot commented Oct 2, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 2, 2026 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 5, 2026

Uh oh!

Uh oh!

codspeed Bot commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Keep module cache entries present during import reordering by youknowdot · Pull Request #8948 · RustPython/RustPython · GitHub

Repository navigation

Keep module cache entries present during import reordering - #8948

Keep module cache entries present during import reordering#8948youknowone merged 3 commits intoRustPython:mainRustPython/RustPython:mainfrom youknowdot:cpython-import-atomic-reorderyouknowdot/RustPython:cpython-import-atomic-reorderCopy head branch name to clipboard

Conversation

youknowdot commented Oct 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Semantic scope

Validation

AI assistance

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Oct 2, 2026

📦 Library Dependencies

Uh oh!

chatgpt-codex-connector Bot commented Oct 5, 2026

Uh oh!

Uh oh!

codspeed Bot commented Oct 5, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Keep module cache entries present during import reordering#8948
youknowone merged 3 commits into
RustPython:mainfrom
youknowdot:cpython-import-atomic-reorder

youknowdot commented Oct 2, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Oct 2, 2026 •
edited
Loading