Skip to content

Modules and imports test panel#94

Draft
tamnd wants to merge 98 commits into
mainfrom
feat/v0.13.5-spec-modules-imports
Draft

Modules and imports test panel#94
tamnd wants to merge 98 commits into
mainfrom
feat/v0.13.5-spec-modules-imports

Conversation

@tamnd

@tamnd tamnd commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Next slice of the spec 1700 vendored-test work: the Modules / imports panel. That's the 12 flat files plus the test_import/, test_importlib/ and test_module/ directory suites, driven to CPython 3.14.5 parity under the 1726 zero-skip bridge (we run what CPython runs and skip what it skips).

Baseline audit against CPython 3.14.5 (all of these are green on CPython):

Test gopy baseline
test_modulefinder ModuleNotFoundError: modulefinder
test_pkg dir() missing cached/doc/loader/spec
test_pkgutil os has no attribute altsep
test_pyclbr ModuleNotFoundError: pyclbr
test_runpy 1 ERROR (test_run_package_init_exceptions)
test_frozen ModuleNotFoundError: hello
test_zipimport os has no attribute altsep
test_zipimport_support os has no attribute altsep
test_zipapp ModuleNotFoundError: zipapp
test__interpchannels / test__interpreters PEP 554, deferred

Plan, smallest blast radius first:

  1. os.altsep + the module-object dir() surface (unblocks pkgutil, zipimport, zipimport_support, pkg)
  2. vendor the pure-Python stdlib modules: modulefinder, pyclbr, zipapp
  3. frozen modules (hello + the frozen table) for test_frozen
  4. the runpy package-init exception residual
  5. re-audit the three directory suites
  6. PEP 554 interpreters, matched to CPython's skip/run behaviour

Spec: website/docs/specs/1700/1731. Opening as a draft; will fill in as each phase lands and keep CI green.

tamnd added 21 commits June 14, 2026 22:13
Audit the 12 flat files plus test_import/, test_importlib/, test_module/
against CPython 3.14.5 under the 1726 zero-skip bridge, and lay out a
phased plan: os.altsep + module dir() surface first, then the pure-Python
stdlib modules (modulefinder, pyclbr, zipapp), frozen modules, the runpy
residual, the directory suites, and finally the PEP 554 interpreters.
The os module published sep/extsep/pathsep but not altsep, so any code
doing os.altsep raised AttributeError. test_pkgutil, test_zipimport and
test_zipimport_support all reach for it through ntpath/posixpath. Add it
to the module constants, matching CPython (None on POSIX, '/' on nt).
All three are pure-Python Lib modules the import panel reaches for, and
all three import cleanly under gopy. test_modulefinder, test_pyclbr and
test_zipapp now get past the ModuleNotFoundError and surface the real
gaps (importlib.machinery.PathFinder, test_importlib package, io
__class__) tracked as follow-ups.
A few gaps that surfaced once test_zipapp could import:

- Python subclasses of the io base types are *Instance objects, so their
  own methods and instance dict have to win over the synthesized native
  methods. The custom getattr/setattr now route those instances through
  the generic path, matching PyObject_GenericGetAttr's MRO walk. This is
  what let _ZipWriteFile override close() and carry _zinfo.
- BytesIO and StringIO keep object's identity hash (they define no
  __eq__), so they're hashable again.
- SystemExit grows its code member, derived from the constructor args
  like SystemExit_init and overridable by assignment.
- os.chmod accepts str, bytes, or any os.PathLike via __fspath__.

test_zipapp now matches CPython 3.14 (35 passing).
test.test_importlib.util guards itself with import_module("_testmultiphase")
at import time, so every test that pulls in that helper (test_pkgutil,
test_pyclbr, the test_importlib extension suites) was raising SkipTest
under gopy where CPython runs them. Reproduce the PEP 489 extension's main
module Go-side: foo, call_state_registration_func, the Example/error/Str
types, and the int_const/str_const constants the C execfunc installs.

Vendor test_importlib into stdlib/test so test.test_importlib.util resolves
as a support module, and re-export importlib.__import__ to match CPython's
public surface (util.py reaches for source_importlib.__import__).

test_pkgutil and test_pyclbr now run their suites instead of skipping; the
remaining failures need the importlib PathFinder/importer surface, which is
the next batch.
pyclbr.readmodule_ex calls importlib.util._find_spec_from_path to locate
a module's source without importing it. Port it on top of the existing
directory-scan helper (factored out of find_spec): check sys.modules
first, returning the cached __spec__ or raising when it is missing/None,
otherwise scan the supplied path. test_pyclbr gets past the import and
now only trips on modules that lack __spec__, which is the next gap.
gopy's import runs Go-side, so modules loaded through PathFinder, the
inittab, and the script-as-main entry never picked up the ModuleSpec
surface CPython's _init_module_attrs fills in. Tools that introspect a
module by name (pyclbr._readmodule, runpy, inspect) read __spec__ and
broke on the missing/None attribute.

Build the spec by calling importlib.util.spec_from_file_location (file
modules) or spec_from_loader (built-ins) once the body has run, mirroring
what FileFinder/BuiltinImporter produce. Modules imported before
importlib.util itself is importable are queued and flushed the moment it
becomes available. The vendored test-as-main module gets a file-location
spec so test.<name> matches what regrtest's import produces; plain
__main__ keeps __spec__ = None like python script.py.

Also force submodule imports for __all__ entries in 'from pkg import *',
and vendor the sre_parse/sre_constants/sre_compile deprecation shims plus
the pyclbr_input fixture.
The machinery.ModuleSpec and util._ModuleSpec stubs diverged from
CPython: no parent property, no has_location/cached descriptors, no
__repr__/__eq__. runpy._run_code reads mod_spec.parent and tools compare
specs, so the stubs broke. Replace both with a faithful port of
importlib._bootstrap.ModuleSpec (gopy keeps it in machinery since the
bootstrap is Go-side) and have importlib.util import it.

spec_from_file_location now follows _bootstrap_external: abspath the
location, set _set_fileattr, derive submodule_search_locations from the
loader. Add _get_cached to _bootstrap_external for the cached property.
The abspath call tolerates posixpath still being mid-import during the
bootstrap spec flush.
Drive test_runpy to parity with CPython 3.14:

- sys.dont_write_bytecode/path_hooks/path_importer_cache top-level attrs
- ModuleNotFoundError carries name= through the import miss path so
  runpy._get_module_details can keep searching dotted names
- subprocess cwd accepts path-like (PyUnicode_FSConverter parity)
- propagate GOPY_STDLIB to child interpreters so subprocess.run with a
  changed cwd still bootstraps encodings
- exit via SIG_DFL SIGINT on unhandled KeyboardInterrupt (bpo-1054041)
- PEP 420 namespace packages on the Go and Python find paths
- _testinternalcapi.get_recursion_depth
Split exitSigint into unix/windows build-tagged files so the windows
runner stops failing on syscall.Kill. Drop the nilerr lint hit in the
ImportError member getter by treating a dict miss as None rather than an
error. Accept os.PathLike argv members in _posixsubprocess.fork_exec the
way fsconvert_strdup does, so subprocess calls passing pathlib.Path args
no longer raise TypeError.
…tartup

gopy reported sys.flags.no_site as 0, claiming the site module had run,
while exit/quit/help/copyright/credits/license were missing. Vendor
site.py and _sitebuiltins.py unchanged from CPython 3.14 and import site
during bootstrap (after encodings) the way init_import_site does, so
site.main() runs setquit/setcopyright/sethelper and the builtins land.

Also fall back to GenericGetAttr in _io.File getattr so dunders like
__class__ resolve through the MRO; abc.__instancecheck__ probes sys.stdout
and was raising AttributeError on the missing __class__.
…d a stale slot

A value-replacement on an at-capacity dict triggers dictResize before the
replace-vs-insert branch in dictInsert. The resize rebuilds the table and
renumbers every slot, but the replace path returns without touching the keys
version, so LOAD_ATTR_INSTANCE_VALUE kept reading the cached (now wrong) slot
index. CPython hands a resized dict a fresh keys object whose dk_version is 0,
which drops every stamped inline cache; mirror that by resetting the version
inside dictResize.

Also route sys.excepthook through the live sys.stderr and format the full
traceback via errors.FormatException, the way _PyErr_Display does, so a test
that mocks sys.stderr captures the output.
ModuleType.__repr__ now forwards to importlib._bootstrap._module_repr the
same way CPython's module_repr goes through _PyImport_ImportlibModuleRepr,
so the __spec__/__loader__/__file__ variants (namespace packages, the '?'
name fallback, bare/full loader reprs) all render identically. Wired the
_bootstrap_external module global that _install_external_importers would
normally set, vendored NamespaceLoader/_NamespacePath, and re-exported
NamespaceLoader from importlib.machinery.

Modules are now GC-tracked with a tp_traverse over md_dict. A module whose
__dict__ holds functions closing over that same dict is a reference cycle;
without the traverse edge the collector treated md_dict as rooted and never
ran __del__ on cyclic objects defined in the module body.

test_module: 39 tests, all green.
zipimport plugs into sys.path_hooks and leans on _bootstrap_external
for the loader machinery. CPython freezes _bootstrap/_bootstrap_external
and runs _setup()/_install_external_importers() at startup to inject sys,
_imp and cross-link the two modules; gopy imports them like ordinary
modules and never runs that startup, so the bindings have to happen at
import time. Bind sys/_imp into _bootstrap, point _bootstrap_external
back at _bootstrap, and have _bootstrap_external register itself as
_bootstrap._bootstrap_external at the end of its module body.

Also fill in the _bootstrap_external pieces zipimport reaches for:
_path_stat, _LoaderBasics, _compile_bytecode, SourcelessFileLoader,
spec_from_file_location, _get_supported_file_loaders and _fix_up_module.
… path hooks

Two fixes that unblock importing modules out of a zip archive on sys.path.

zlib.Compress.flush() defaulted to Z_SYNC_FLUSH, but CPython defaults the
mode to Z_FINISH. The common compressobj().compress(x) + flush() idiom has
to emit a complete deflate stream (final block) or a one-shot decompressor
reads back a truncated stream and raises 'unexpected EOF'. zipfile stores
compressed members through that idiom and zipimport inflates them with raw
deflate, so every compressed-zip import was failing.

Add sys.meta_path so import_helper's save/restore around each test stops
raising AttributeError, register zipimport.zipimporter on sys.path_hooks
ahead of the FileFinder hook, and have the Go path finder consult
sys.path_hooks for non-directory sys.path entries: it builds the importer,
asks it for the spec, and loads the module via module_from_spec +
exec_module, mirroring _bootstrap._load_unlocked.
PEP 420 namespace packages were dropping portions found in zip path-hook
importers, so a package split across two archives ended up with a
__path__ of length 2 instead of the merged single entry. PathFinder now
accumulates namespace portions from path-hook specs the same way it does
for plain directories.

A real-filesystem namespace portion that only holds .pyc files (no .py)
was also invisible to the directory scan, so submodules under it raised
ModuleNotFoundError. Added __init__.pyc and <tail>.pyc handling that
loads the marshalled code directly.

importlib.util.module_from_spec was a divergent stub that only set
__file__ when origin was not None; namespace specs left it unset and
mod.__file__ raised AttributeError. Re-export _bootstrap.module_from_spec
so namespace modules get __file__ = None like CPython.

zlib.crc32/adler32 now accept bytearray.

Drops two test_zipimport scratch artifacts that were committed by mistake.
…Error.msg

gopy compiles every extension module into the binary, so they behave
exactly like statically-linked builtins: they are found before the path
finder and cannot be shadowed by a module of the same name on sys.path.
sys.builtin_module_names only listed builtins and sys, which left the
importlib builtin/extension finder tests with no usable module name and
made test_zipimport.testAFakeZlib run (and fail) instead of skipping the
way it does on a statically-linked CPython build. Build the tuple from
the inittab snapshot, minus the few pure-Python modules gopy keeps there
as an import shortcut so 'os' in sys.builtin_module_names stays False.

ImportError now exposes the msg member CPython sets from the single
positional argument, so exc.msg (read by zipimport's bad-magic test and
others) works.
func_getattro pulled the attribute straight out of the function __dict__
and returned it without an incref, so the caller's arg-drop could decref a
value the dict still held. A list stored on a function (mock wraps its
patchings list on the decorated function this way) got emptied by
list_dealloc after the first read, so a second read saw an empty list and
the shared decorator silently stopped patching across test classes.

Matches PyXINCREF in Objects/funcobject.c func_getattro.
@tamnd

tamnd commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

test_zipimport is fully green now (91 tests, 4 skipped to match CPython).

Two things were behind the last failures:

  • The two testTraceback errors were just a missing _testcapi.config_get. Ported it (plus config_getint/config_names) over a PyConfig_Get spec table.
  • test_checked_hash_based_change_pyc was the interesting one. It only failed in the cross-class run, and it turned out not to be a zipimport bug at all. func_getattro was reading attributes straight out of a function's __dict__ and returning them without an incref. mock.patch as a decorator stashes its patchings list on the wrapped function, so after the first test consumed it the list got emptied by list_dealloc, and the second class that shared the inherited decorated method silently stopped patching. Added the Py_XINCREF that CPython does. Minimal repro was just f.lst = [1]; print(f.lst); print(f.lst) printing [1] then [].

That incref fix is broad, not zipimport-specific, so worth a look.

Where the rest of the panel stands: test_module/ is green (39 tests). test_import/, test_importlib/, test_modulefinder, and test_runpy all bottom out on the same thing: our import dispatch runs Go-side and sys.meta_path is empty, so the Python PathFinder/FrozenImporter/BuiltinImporter finders aren't live and importlib.machinery doesn't re-export them. Making those the real dispatch path (and porting the _imp C functions the full bootstrap needs) is its own subsystem-sized piece. Wrote it up as P7 in the spec.

tamnd added 8 commits June 15, 2026 02:38
gopy shipped a trimmed importlib (stub machinery.py, a util.py that
imported source_hash from _bootstrap_external instead of defining it,
a _bootstrap.py that injected sys/_imp at module top). sys.meta_path
was empty and the Python finders were dead code, so anything that
introspected the import system or walked meta_path failed.

Vendor the unmodified CPython 3.14 files (__init__, _bootstrap,
_bootstrap_external, util, machinery, _abc, abc) plus the metadata,
resources, readers and simple submodules, then run the two-phase
install at startup the way pylifecycle does: __init__ self-bootstraps
through its except-ImportError branch (we have no frozen
_frozen_importlib), then we call _bootstrap._install /
_bootstrap_external._install directly so meta_path ends up as
[BuiltinImporter, FrozenImporter, PathFinder] and the FileFinder /
zipimport path hooks are registered.

Port the _imp C-function surface the full bootstrap drives
(find_frozen, get_frozen_object, is_frozen_package, create_builtin,
exec_builtin, extension_suffixes, _fix_co_filename) and add
sys.pycache_prefix so cache_from_source works.

test_runpy goes green (40), test_pkg green, test_pkgutil down to a
couple of residuals.
…mpare equal

ImportModuleLevel now walks sys.meta_path for finders a program installs,
skipping the BuiltinImporter/FrozenImporter/PathFinder entries gopy realizes
in Go and driving any spec a custom finder returns through loadFromSpec. This
lets test-installed importers (pkgutil's MyTestImporter) satisfy 'import foo'.

classmethod_get now stamps methOrigin so two bindings of int.from_bytes compare
equal and hash alike, matching meth_richcompare's m_ml pointer test.
Port _PyModule_IsPossiblyShadowing to read the startup-captured leading
sys.path entry (config->sys_path_0) instead of live sys.path[0], so a
script that mutates sys.path after startup keeps consistent shadowing
detection. The leading entry is now prepended to sys.path after site.main
runs, matching CPython, so a -c run keeps sys.path[0] == '' rather than
letting site.removeduppaths absolutize it.

Set spec._initializing around module exec so a self-importing module hits
the circular-import and 'consider renaming' hints. Pass the live __name__
object through to PySet_Contains so an unhashable str subclass raises, and
guard stdlib_module_names with PyAnySet_Check. Module getattro now formats
with %U-style literal quotes; os.__getattr__ miss uses single quotes.
A backward jump computed off an instrumented bytecode position read the
live byte (INSTRUMENTED_LINE or an INSTRUMENTED_<X> variant) and looked
its cache width up in the per-opcode table, which is keyed by base
opcode only. The marker returned a zero cache count, so the jump landed
one codeunit short of its target. Under sys.settrace this dropped the
loop header by one instruction in inlined comprehensions, leaving the
freshly built list on top of the stack instead of the iterator and
raising 'list object is not an iterator' on the next FOR_ITER.

advance() now resolves the marker (via the line original-opcode table)
and de-instruments before reading the cache stride.
CPython binds the builtins module object (not its dict) to __builtins__
in the __main__ namespace; every other module gets the dict. The frame
builder already unwraps a module back to its dict for LOAD_GLOBAL, so
the only behavioural change is that 'del __builtins__.__import__' now
reaches a module attribute and the import machinery raises ImportError
afterwards, matching test_import.test_delete_builtins_import.
The integer-fd and path-open FileIO constructors wrap an os.File whose
descriptor gopy already owns through FileIO.Close and the closefd flag.
Go's runtime also arms its own finalizer on those os.File values, and a
GC mid-run could fire it after the descriptor number had been freed and
reused by an unrelated open file, closing that file's fd out from under
it. Long write loops then failed with a spurious EBADF.

Clear the Go finalizer at every borrowed-fd wrap site, and also on
os.isatty's throwaway wrapper, so release stays deterministic.
…ystems

On macOS the filesystem is case-insensitive but case-preserving, so a
plain os.Stat probe lets `import RAnDoM` resolve random.py. CPython's
FileFinder guards against this by testing the candidate name against the
exact-case set(os.listdir(dir)) unless _relax_case() allows folding.

Port that check: confirm each resolved candidate's final component matches
a real directory entry with exact case, relaxed only on case-insensitive
platforms when PYTHONCASEOK is set.
tamnd added 5 commits June 16, 2026 09:55
os.NewFile/os.OpenFile arm the close finalizer on the unexported inner
*os.file, so SetFinalizer(f, nil) on the outer handle never disarmed it.
A leaked borrowed-fd wrapper (subprocess pipes, fdopen) would then close
a descriptor whose number had already been reused, surfacing as a
spurious 'bad file descriptor' on an unrelated write. test_import's
test_module_with_large_stack hit this during its GC-heavy write loop.

Reach the inner pointer and clear the finalizer there, behind a shared
objects.ClearOSFileFinalizer, and route the io and _posixsubprocess
borrows through it. Drop a dead charmapDecode, refactor buildSpec under
the complexity gate, and fix the stale fork_exec test that expected a
tuple where CPython returns a plain pid.
The int64() wraps on Atim/Mtim/Ctim Nsec and on Blksize/Blocks are
redundant on linux/amd64 (the CI lint host) but necessary on 32-bit
linux where those syscall.Stat_t fields are int32. Match the existing
//nolint:unconvert pattern already used for Nlink and Blksize above so
the linux lint job stays green without dropping 32-bit portability.
run() reuses one process-wide sys.modules across invocations, while
CPython gets a fresh interpreter per process and runs init_importlib
exactly once. The cmd/gopy tests call run() several times in a single
test binary: the first call installs the import system and aliases
_frozen_importlib to the source _bootstrap module (which has no
__origname__), and the next call re-ran _bootstrap._install, so _setup
re-scanned sys.modules and tripped the frozen fix-up assert
('see PyImport_ImportFrozenModuleObject()') on that aliased module.

Guard the install on '_frozen_importlib' not in sys.modules so it runs
once, matching CPython. The standalone binary was unaffected (one
bootstrap per process) which is why it only surfaced under go test.
@tamnd

tamnd commented Jun 16, 2026

Copy link
Copy Markdown
Owner Author

Pushed two fixes that were keeping CI red.

First, the lint job on the Linux runner was flagging six conversions in module/os/stat_linux.go as redundant. They're only redundant on amd64. Stat_t.Atim.Nsec, Blksize, and Blocks are int64 there but int32 on 32-bit arches, so the int64(...) wraps are needed for the build to stay portable. My local lint never saw them because the _linux.go files are build-excluded on macOS. Annotated each with //nolint:unconvert and a note on which field is narrower elsewhere, matching the two lines that were already annotated for Nlink/Blksize.

Second, the cmd/gopy test job was failing on every OS with preload importlib: AssertionError: see PyImport_ImportFrozenModuleObject(). The test binary calls run() several times in one process, and we reuse a single process-wide sys.modules across all of them, where CPython gets a fresh interpreter per process and runs the importlib bootstrap exactly once. So the second run() re-executed _bootstrap._install over a sys.modules that already had _frozen_importlib aliased to the source _bootstrap module, and _setup tripped the __origname__ assert in _fix_up_module. Wrapped the install in if '_frozen_importlib' not in sys.modules: so it runs once per process like CPython does. No stdlib changes for this; the fix is in the bootstrap driver in main.go.

CI is green across lint, vet, cfg-phase-parity, and test on ubuntu/macos/windows.

test_runpy, test_pkg, test_pkgutil, test_modulefinder, test_pyclbr, and
test_zipapp all run green now; record them and narrow the remaining
residuals to the test_import C-ext subinterp errors and the two
test_importlib GC/threading edge cases.
@tamnd

tamnd commented Jun 16, 2026

Copy link
Copy Markdown
Owner Author

Re-audited the imports panel against CPython 3.14 now that CI is green. A lot of it has come along since the spec table was written.

Green now, with the test counts:

  • test_runpy: 40 tests, the package-init exception case is fixed
  • test_pkg: 8 tests
  • test_pkgutil: 21 tests
  • test_modulefinder: 17 tests
  • test_pyclbr: 6 tests
  • test_zipapp: 35 tests
  • test_module/: 39 tests
  • test_frozen: 3 tests
  • test_zipimport: 91 tests (4 skipped, and it's slow because the Zip64 large-file case really does write a multi-GB archive)

What's left:

  • test_import/ runs all 118 tests, 16 skipped, 5 errors. The 5 are the _testsinglephase/_testmultiphase C-extension subinterpreter tests. That's the P7 work, we don't have those test C extensions yet.
  • test_importlib/ runs 1346 tests, 63 skipped, down to 2 failures and 1 error. The error is the same multi-phase C-ext path. The two failures are GC/threading edge cases: test_all_locks wants _bootstrap._module_locks to drain to zero after a collect (our collector leaves the dead _ModuleLock weakref entries around across the full sweep, even though the isolated test_lock_lifetime passes), and test_circular_imports is a threaded-import ordering case. Both sit on top of the broader weakref/GC work rather than anything import-specific.

Updated the spec table and checklist to match. Still chipping at the C-ext and GC residuals.

tamnd added 4 commits June 16, 2026 18:39
Build out the single-phase-init extension subsystem the test_import
panel drives through ExtensionFileLoader:

- imp/extension.go: extensions cache keyed by (path, name) with the
  three module kinds (m_size -1 basic, 0 reinit, >0 with-state), reload
  via m_copy for basic, modules_by_index per interpreter backing
  look_up_self, and clear_extension for teardown restore.
- _testsinglephase: the basic / basic_wrapper / basic_copy / with_reinit
  / with_state / check_cache_first / raise_exception / circular variants,
  matching the PyInit_ entry points the .c module exposes.
- _testsinglephase_circular imports its helper from PyInit before adding
  itself to sys.modules and caches itself in a static var (gh-123950).
- PyImport_ImportModule now drives the live importlib _gcd_import so a
  deeply dotted namespace-package helper resolves with its parents,
  which the Go-only ImportModule driver did not import.
- _testinternalcapi.clear_extension wired to _PyImport_ClearExtension.

test.test_import is down to the multi-interpreter isolation cases that
need real per-interpreter sys.modules.
A fresh subinterpreter starts with an empty sys.modules, so importing an
extension there re-runs the import (firing the PEP 489 compat gate) even
when the main interpreter already cached it. gopy shares one sys.modules
dict across the synchronous subinterpreter stack, so the compat check
never saw the re-import and the singlephase tests all returned 'okay'.

PushSubinterp now hides the registered extension entries for the duration
of a run and PopSubinterp restores them, and run_in_subinterp pushes a
legacy interpreter state so its script re-imports through the cache. Gets
the SubinterpImportTests compat checks and the basic_multiple_interpreters
snapshots to match CPython.
@tamnd

tamnd commented Jun 16, 2026

Copy link
Copy Markdown
Owner Author

test_import/ is green now. All 118 tests pass, 4 skipped, CI green on every runner.

Getting there meant porting the single-phase extension machinery the SubinterpImportTests lean on, since the _testsinglephase* family was the bulk of what was still erroring:

  • The extensions cache from Python/import.c, keyed by (path, name), with the three m_size kinds: -1 "basic" (process-global state, cached, reloaded by copying m_copy rather than re-running init), 0 "reinit", and >0 "with state". The _testsinglephase variants, the def-sharing wrapper, and the *_check_cache_first cases all hang off that.
  • The gh-123950 circular import. _testsinglephase_circular caches itself in a process-global before adding itself to sys.modules, then imports a helper that re-imports it. I wired it through an import hook that drives _frozen_importlib._gcd_import, because the Go-side import driver wasn't recursing parent packages and the helper lives several levels deep in a namespace package.
  • Per-subinterpreter sys.modules. This was the subtle one. CPython gives every interpreter its own sys.modules, so importing an extension in a subinterpreter re-runs the import and fires the PEP 489 compat gate even when the main interpreter already cached it. gopy runs subinterpreters synchronously against one shared sys.modules, so the gate never saw the re-import and every compat check returned 'okay'. Now PushSubinterp hides the registered extension entries for the duration of a run and PopSubinterp restores them, and run_in_subinterp pushes a legacy interpreter state so its script re-imports through the cache. That fixed the four check_singlephase_setting_and_override cases and both basic_multiple_interpreters_* snapshot tests in one go.

Two follow-up pushes kept CI green: the lint job flagged the extension stub-file writer (gosec wants 0750/0600 perms) and a few analogue spellings.

Of the 4 skips: three are platform-specific (Windows DLL/UNC paths, TESTFN_UNENCODABLE), same as CPython. The fourth is test_frozen_compat, which wants _frozen_importlib.__spec__.origin == 'frozen'. gopy loads importlib from source, so that one waits on the frozen-importlib work in P7.

test_importlib/ is unchanged (18 failures + 44 errors, the _testmultiphase multi-phase loader, tracked separately); test_module/ stays green at 39.

tamnd added 3 commits June 17, 2026 11:26
Wire the PEP 489 subinterpreter compatibility check through _interpreters.exec
so single-phase and NOT_SUPPORTED extension modules raise ImportError when
loaded into a non-main interpreter with the check enabled, matching CPython.
Read the live pending exception in excinfoFor so the snapshot carries the real
ImportError type name across the VM reraise wrapper.

Set name/msg on the ImportError raised by the dynamic-load fallback in _imp so
test_unloadable sees cm.exception.name.

Build _testmultiphase's Str(str) subtype lazily at exec time instead of Go init
time: str's tp_new is wired by builtins during interpreter startup, so a type
built at package init copies a nil tp_new and produces non-str-backed instances.
The fresh-tuple inits built their args with NewTuple (refcount 1, no
other owner) and then went through setArgs, which takes a second
counted reference for borrowed tuples. That left the tuple, and through
it args[0], pinned at +1 after the exception was gone, so weakref-based
leak checks on the arguments never saw the object drop.

Split the two cases: setArgsSteal transfers the single ref a freshly
built tuple already holds, setArgs keeps increfing for the borrowed
path. excTpNew now allocates with empty args and lets tp_init install
the real tuple, and the KeyError factory builds its args directly.

Fixes the Source_LifetimeTests.test_all_locks lock-lifetime check.
When dispatch raises and no handler in the frame catches it, CPython's
exception_unwind drops every remaining operand-stack temporary before
the frame is popped. gopy deferred that to frame.Clear, which runs after
chunk.Pop has already snapshotted the frame for any live tb_frame. A
traceback attached during the same unwind wraps that frame, so a stale
exc-info temporary still on the stack got copied into the snapshot and
pinned the frame's locals, e.g. the _ModuleLockManager from a failed
import, long after the exception was gone. Clear the stack here so the
snapshot only captures fast locals, cells and frees.

Also give the frame a counted reference on its function object for the
duration of the call and drop it in Clear, matching f_funcobj handling,
and snapshot the finalizer node list before running finalizers so a
tp_clear cascade that unlinks a node mid-walk can't deref a stale next.
@tamnd

tamnd commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Update on the lock-lifetime test.

Frozen_LifetimeTests.test_all_locks asserts _module_locks is empty after a gc_collect(), and we're still holding ~41 weakrefs for the startup modules (site, os, re, enum, collections, unittest, ...). I chased this all the way down and it's not a leak in the importlib code, it's our refcount model.

The chain: _ModuleLock.acquire() runs with _BlockingOnManager(tid, self):. The manager temporary stashes the lock in its __dict__. The manager does hit refcount 0 and dealloc, but our baseline instanceDealloc only decrefs the type, it never releases the instance __dict__ contents. So every acquire/release leaks one phantom ref onto the lock (confirmed: 10 cycles -> +10). ~21 phantom refs pile up per startup lock, so the cyclic collector computes gc_refs > 0, never reclaims, and the weakref callback that does del _module_locks[name] never fires.

Porting CPython's subtype_dealloc (clear __dict__ on dealloc) fixes the lock leak cleanly. But it's destructive, and it immediately trips every object we hand to a container that doesn't count its contents. The first one is contextvars: the persistent HAMT stores values with no incref, so the _Context that warnings keeps in a ContextVar drops to refcount 0 while still reachable, gets its dict wiped, and you get '_Context' object has no attribute '_filters'.

I tried to close the HAMT gap at the contextvars layer (incref on Set, decref on Reset/Without) but it's unsound: Context.Copy() shares HAMT versions, so one context's reset would decref a value another context still references through the shared nodes. The correct fix is node-level refcounting like CPython's hamt.c (a value dies only with its last sharing node), which needs synchronous node death our Go-GC'd nodes don't have, plus teaching the cyclic collector to traverse HAMT nodes.

So this one test sits on top of a real refcount-accuracy investment. I've reverted the destructive change to keep the branch green and the rest of the panel intact, and I'm picking up the tractable imports-panel work next while we decide how far to push the refcount audit.

tamnd added 5 commits June 18, 2026 02:57
Port hamt.c's owned-reference discipline: each assoc/without returns an
owned node, clones incref every copied slot, freshly built children
transfer ownership into the slot, and each node's dealloc decrefs the
slots it holds. The empty bitmap stays immortal. This lets a value held
only by the context HAMT keep an accurate refcount, so it survives a
destructive instance __dict__ clear instead of being torn down early.
subtype_dealloc clears the instance dict on teardown; gopy was only
decrefing the type, so attribute values stayed pinned by a refcount no
live object accounted for. Add ClearOwnedContents and call it from
instanceDealloc for an unexposed dict (a dict handed to Python may be
aliased, so that case is left to its own owner).
Set/Reset go through a Py_SETREF helper, the context/var/token deallocs
release what they hold, and the cyclic-collector traverse visits them.
ContextVar.get, Context.get and ctx[var] now return a new reference like
PyContextVar_Get does, so repeated lookups of a value held only by the
HAMT no longer drive it below its true refcount.
These are T_OBJECT members in CPython and member_get increfs before
returning. Handing back the bare field leaked a borrow the VM later
decrefs, so a bound method held only through a container (e.g. an
ExitStack callback) tore down its instance while still reachable.
The dict-release I added in 88c6d2c ran from instanceDealloc, i.e. the
eager refcount-zero path. That is unsound here: the VM still under-counts
some borrowed refs, so a live object can reach refcount zero and get its
__dict__ wiped out from under code that is still using it. It showed up as
_ZipWriteFile losing _file_size mid-write, _Context losing _filters, and
similar AttributeErrors across test_set, test_runpy, test_pkgutil and
test_importlib/test_abc.

CPython does this clear in delete_garbage via tp_clear, which only runs
after the cycle collector has proven the object unreachable. Port that
shape: add a TpClear slot, wire subtype_clear for pure user classes, and
call it from a clearGarbage pass in collectMain. The pass runs with the
collector mutex dropped (like finalizeGarbage) because tp_clear decrefs
members and a member hitting zero re-enters Untrack.

Also stamp the header finalized bit when the collector runs a finalizer,
not just the gc-layer flag, so the member decref during the clear pass
cannot re-fire __del__ on an object that was already finalized.

test_all_locks still passes (the clear breaks the lock cycles at GC time)
and the regressions above are gone.
@tamnd

tamnd commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Tracked down the test_set / test_runpy / test_pkgutil / test_importlib.test_abc breakage to the __dict__ release I added a few commits back. That release was running from instanceDealloc, i.e. the eager refcount-zero path, and the VM still under-counts a handful of borrowed refs, so a live object could hit zero and get its dict wiped while it was still in use. The clearest symptom was _ZipWriteFile losing _file_size mid-write and the warnings _Context losing _filters.

Moved the clear to where CPython does it: delete_garbage via tp_clear, which only runs after the cycle collector has proven the object unreachable. Added a TpClear slot, wired subtype_clear for user classes, and call it from a clearGarbage pass in collectMain with the collector mutex dropped (tp_clear decrefs members and a member hitting zero re-enters Untrack). Also had to stamp the header finalized bit when the collector runs a finalizer so the member decref during the clear pass can't re-fire del on an already-finalized object.

test_all_locks still passes (the lock cycles break at GC time as before) and the regressions are gone:

  • test_set OK
  • test_runpy / test_pkgutil / test_importlib/test_abc OK
  • test_zipimport OK (skipped=4)
  • test_contextlib down to the pre-existing test_nokeepref

test_namespace_pkgs still has its two pre-existing issues (json import under invalidate_caches, and an absolute-vs-relative path), same as before this change. Looking at those next.

tamnd added 6 commits June 18, 2026 03:36
The CI lint gate flagged three issues on this branch: a misspelled
'behaviour' in errors/builtins.go, the always-true bool result on
runSinglephase/reloadSinglephase, and fromDefAndSpec creeping over the
cognitive-complexity ceiling.

The two singlephase helpers always returned found=true once we had a
def, so the bool carried no information. Drop it and let CreateExtModule
supply the constant true at the call sites. Extract the slot-table scan
in fromDefAndSpec into scanExtSlots, which both reads cleaner and pulls
the function back under the gocognit limit.
…un-mode note

test_all_locks now passes: the module-lock drain landed when the
collector grew a tp_clear slot driven from delete_garbage. Drop it from
the residuals and note the namespace_pkgs standalone-vs-package run-mode
artifact, which CPython 3.14 reproduces identically.
PyImport_ImportModuleLevelObject checks sys.modules before ever calling
_find_and_load. When the cached module is still initializing (another
thread is mid-import on it), the C body waits on the per-module lock via
_bootstrap._lock_unlock_module, which catches the _DeadlockError a
concurrent circular import raises. _find_and_load's own _ModuleLockManager
lets that error propagate and kills the importing thread, so going
straight there for an in-flight circular import is the difference between
both threads finishing and one dying.

Route IMPORT_NAME through importModuleLevelObject, which now takes that
fast path only for the cached-and-initializing case and delegates every
other import to the live __import__ (its _find_and_load already returns a
fully loaded module without locking, so the common path is unchanged).
The cache hit borrows from sys.modules where import_get_module returns a
new reference, so incref before handing the module back or IMPORT_NAME's
DECREF_INPUTS under-counts it.

Fixes test_threaded_import.test_circular_imports.
The circular-import cache fast path and the gh-134100 dotted-head KeyError
pulled the shared importModuleLevelObject in opposite directions: the VM
IMPORT_NAME opcode needs the refcount-proven delegateImport route (it applies
DECREF_INPUTS to the module it pushes), while the builtin __import__ needs the
C-faithful _gcd_import + headSelection that raises KeyError when a non-module
sits in sys.modules.

Give IMPORT_NAME its own importViaDelegate: it runs the same
import_ensure_initialized still-initializing fast path (so concurrent circular
imports resolve instead of dying on an uncaught _DeadlockError) but otherwise
delegates the load to _frozen_importlib.__import__. The accepted fast-path
branch still runs the fromlist / dotted-head selection so a bare
'from . import sub' during a package's own init force-imports the submodule.
importModuleLevelObject keeps the full C body for the builtin __import__.
Both the threaded circular-import failure and the incomplete multi-phase init
error are resolved; the panel runs 1346 tests with 0 failures and 0 errors.
A cached module without a usable __spec__ cannot be mid-import, so
acceptInitializingModule treats the lookup failure as not-initializing and
falls back to the normal import path. Annotate the deliberate error swallow so
the nilerr linter passes.
@tamnd

tamnd commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

The two remaining test_importlib residuals are closed, so the panel now runs 1346 tests with no failures and no errors.

The threaded circular-import case was the interesting one. The fix is splitting the two import entry points that had been sharing a single function:

  • The VM IMPORT_NAME opcode keeps the long-standing _frozen_importlib.__import__ delegate route. It applies DECREF_INPUTS to the module it pushes, and that route returns a reference whose count the decref is balanced against. What it gains is the import_ensure_initialized still-initializing fast path out front: when another thread is mid-import on the target, it waits via _bootstrap._lock_unlock_module (which catches _DeadlockError) instead of re-entering _find_and_load's _ModuleLockManager, which lets the error propagate and kills the importing thread. That's the difference between both threads finishing a circular import and one dying.

  • The builtin __import__ keeps the full PyImport_ImportModuleLevelObject body: _gcd_import plus the C dotted-head selection that re-reads sys.modules and raises KeyError when a non-module has been stuffed in there (the gh-134100 guard). Routing that through the Python mirror would surface an AttributeError instead.

I tried to keep these as one function and it kept pulling in opposite directions: the opcode needs the delegate route for refcount reasons, the builtin needs the C-faithful head selection for the KeyError. Splitting them is cleaner than trying to make one body satisfy both.

Verified with a per-module diff of the whole test_importlib tree against the pre-change binary: the only delta is test_threaded_import going from one failure to zero. Everything else is byte-identical, including the known GC-off and PathFinder run-mode artifacts. CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant