fix(runtime): fence the dense subclass seqlock; specialize imported this-methods and proven Symbols - #8737
Conversation
…his-methods and proven Symbols Lands #8735, #8729 and #8731. #8735 fixes a real data race in the dense-array subclass layout cache. `cached_dense_layout` read two payload fields `Relaxed` and then rechecked the sequence counter with an `Acquire` LOAD -- but an acquire load only constrains what follows it, so on weakly ordered hardware the preceding payload reads could sink past the recheck. A reader racing a colliding publisher could then combine one field from the old layout with one from the new and return a WRONG ELEMENT rather than faulting. A standalone `fence(Acquire)` now sits between the payload reads and the recheck, which is the canonical seqlock-reader form. #8729 lowers strict equality against a proven Symbol to raw NaN-boxed identity, keeping loose equality, reassigned locals and erased TypeScript annotation claims on the semantic runtime helpers. #8731 (fixes #8693) publishes producer-authoritative proven-`this` method capabilities through imports, aliases and re-exports, emitting guarded direct imported clone calls while retaining generic dispatch fallbacks. A changelog fragment was added for #8735, which had neither one nor a skip-changelog label. No version bump.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (42)
📝 WalkthroughWalkthroughThe PR adds imported ChangesImported method specialization
Symbol identity equality
Runtime memory ordering
Estimated code review effort: 5 (Critical) | ~90 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ImportedModule
participant CompilePipeline
participant Codegen
participant Runtime
ImportedModule->>CompilePipeline: publish proven-this method capabilities
CompilePipeline->>Codegen: attach capabilities to ImportedClass
Codegen->>Runtime: emit guarded pshape direct call
Runtime->>Codegen: use generic dispatch fallback when guards fail
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Lands #8735, #8729 and #8731.
#8735 — fence the dense subclass seqlock
A real data race, and the reasoning checks out.
cached_dense_layoutread two payload fieldsRelaxed, then rechecked the sequence counter with anAcquireload — but an acquire load only constrains operations that follow it, so on weakly-ordered hardware the preceding payload reads could sink past the recheck. A reader racing a colliding publisher could combine one field from the old layout with one from the new and return a wrong element value rather than faulting, which is the worst failure shape.The fix is a standalone
fence(Acquire)between the payload reads and the recheck — the canonical seqlock-reader form, and precisely the ordering an acquire load cannot provide:#8729 — compare proven Symbols by identity
Lowers strict equality against a proven Symbol to raw NaN-boxed identity, while keeping loose equality, reassigned locals, and erased TypeScript annotation claims on the semantic runtime helpers. That last exclusion is the important one: a declared type is not a proof.
#8731 — specialize imported methods capturing
this(fixes #8693)Publishes producer-authoritative proven-
thismethod capabilities through imports, aliases and re-exports, emitting guarded direct imported clone calls with generic dispatch fallbacks retained.Validation (on the merged result)
lint-job checkers passperry-runtime --lib(RUST_TEST_THREADS=1): 2669 passed, 0 failedperry-codegen --lib: 1222 passed, 0 failed (+8)perry-codegen --tests(all integration suites): 0 failuresA
changelog.d/fragment was added for #8735, which had neither one nor askip-changeloglabel.No version bump.
Summary by CodeRabbit
Performance
this, while preserving correct fallback behavior.Symbol()andSymbol.for()values.Bug Fixes
Tests