Give the strong count the bits an unused weak field held - #8574
Conversation
RefCount packs its state into one usize and splits the non-flag bits evenly between a strong and a weak count, so a 32-bit target left the strong count 15 bits. An object reaching 32 767 references then called refcount_overflow, which aborts the process; on wasm that surfaces as an unreachable trap with no Python-level error. Two ordinary module imports pass that total. The weak half is never read and never written. WEAK_COUNT appears once outside its own definition, in RefCount::new, and nothing in the repository observes those bits: weak references live in the object's WeakRefList, which Py::weak_count walks. The strong count now takes every bit the three flags leave, which is 29 on a 32-bit target and 61 on a 64-bit one. The tests run on wasm32-wasip2, where the old layout traps and the new one passes. A 64-bit host passes either way, since 31 bits already covered them. Fixes RustPython#8469 Assisted-by: Claude Code:claude-opus-5
The count is 61 bits wide on a 64-bit target once it takes the bits the weak field held, so a u32 return truncates above 4 294 967 295 references: inc would read a wrapped 0 and re-increment, and dec would read a wrapped 1 and free a live object. A 32-bit target is unaffected, where 29 bits fit u32. Assisted-by: Claude Code:claude-opus-5
inc_by writes its own overflow check, so a test built on it leaves inc and dec unexercised. The reported aborts came through inc, one reference at a time. Assisted-by: Claude Code:claude-opus-5
|
No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. π WalkthroughWalkthroughThe refcount state now uses all non-flag bits for the strong count. Initialization no longer adds an implicit weak reference. Strong-count operations use native ChangesReference-count state expansion
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: βͺ Minimal Β· up to This localized change expands the available strong-reference count on 32-bit targets without increasing object size, and the supplied test and build results cover the affected wasm and runtime paths. No actionable merge-blocking risk remains beyond normal checks and review. π₯ Pre-merge checks | β 5β Passed checks (5 passed)
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
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. Comment |
Fixes #8469.
The first two commits are @JMLX42's, cherry-picked with authorship intact from
https://github.com/JMLX42/RustPython/tree/fix-32bit-refcount-ceiling β they reported the bug and
wrote the fix.
The bug
RefCountpacks its state into oneusizeand split the non-flag bits evenly between a strong anda weak count, leaving a 32-bit target 15 bits of strong count. Every object holds a strong
reference to its type, so 32 767 live instances of one type overflowed that type object's count and
called
refcount_overflow()βstd::process::abort(), which surfaces on wasm asRuntimeError: unreachablewith no Python-level error and no message.The weak half was never read and never written:
WEAK_COUNTappeared only in its own definitionand in
RefCount::new, and weak references are counted by walking the object'sWeakRefList(
Py::weak_count). Giving the strong count every bit the three flags leave takes 32-bit targetsfrom 32 767 to 536 870 911 and 64-bit targets to 2^61 - 1. The word stays
usize, so no objectheader grows and no target needs a 64-bit atomic.
The second commit widens
State::strongtousize: at 61 bits au32return truncates above2^32, where
incwould read a wrapped 0 and increment again, anddecwould read a wrapped 1 andreport a live object collectable.
The third commit adds a test through
inc/dec.incwrites its overflow check separately frominc_by's, and it is the path the reported aborts came through βContext::intern_strβstr_type.to_owned()βinc.Verification
Built
wasm32-wasip1(wasm-release,--features freeze-stdlib,stdlib,stdio,importlib,host_env)before and after, and ran the reproducer from #8469 under wasmer 7.3.0:
holder = [marker] * NRuntimeError: unreachable, exit 45RuntimeError: unreachable, exit 45That matches the bisection in the issue exactly.
Lib/test/test_list.pytrapped on its third testbefore and now runs all 68 to completion;
test_int,test_dictandtest_setpass, as do bothsnippets the
wasm-wasiCI job runs.cargo test -p rustpython-common --target wasm32-wasip2under wasmtime: 4 passed. The ceilingtests are no-ops on a 64-bit host, where 31 bits already covered them, so a 32-bit target is
where they mean anything.
cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: passcargo testincrates/capi: 102 passedcargo run --release -- -m test test_gc test_weakref test_list test_dict test_set test_types test_sys: 1 238 run, SUCCESScargo clippy -p rustpython-common --all-targets -- -Dwarnings: cleancargo check --target wasm32-wasip1, with and withoutthreading: cleanπ€ Generated with Claude Code
Summary by CodeRabbit