fix(runtime): share hot TLS state across provider images - #9805
fix(runtime): share hot TLS state across provider images#9805proggeramlug wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe runtime now shares thread-local provider storage by named declaration identity. Provider tests cover storage reuse and local declaration separation. The provider fixture retains additional stream and response symbols, and the changelog records the fixes. ChangesProvider runtime compatibility
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to Provider loading symbols and shared TLS storage behavior are covered by the updated implementation and focused tests, with no unresolved merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant ProviderCopyA
participant js_tls_hot_claim_slot
participant SharedCache
participant ProviderCopyB
ProviderCopyA->>js_tls_hot_claim_slot: claim named declaration slot
js_tls_hot_claim_slot->>SharedCache: publish slot identity
ProviderCopyA->>SharedCache: initialize and publish value
ProviderCopyB->>js_tls_hot_claim_slot: claim the same named declaration
js_tls_hot_claim_slot-->>ProviderCopyB: return shared slot
ProviderCopyB->>SharedCache: read published value
SharedCache-->>ProviderCopyB: reuse cached value
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 52.94% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 4 files. (1 skipped: 1 unsupported.)
✨ 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 |
|
Landed on |
The native-root provider gate could not load its streamed Response app because the standalone stdlib omitted two required helpers. Retaining/exporting those helpers exposed a second failure: separately built runtime copies assigned unrelated thread-local declarations to the same shared HotTls slot. LLDB showed CURRENT_IMAGE and STATE_PTR both claiming slot 0, corrupting class-image state during module initialization.
Retain/export the strategy-aware ReadableStream constructor and Response body-init reset helper. Give hot TLS declarations stable identities through a shared, non-inlined C registry entry point, and reuse already-published storage before initializing a second provider copy. The cache layout and hot read path remain unchanged. As with the existing provider ABI, the images must come from compatible source builds.
Closes #9791.
Validation on macOS arm64:
Linux provider validation remains for CI. The previous targeted CI run was canceled while queued during release-runner prioritization; it did not execute. No version bump.
Summary by CodeRabbit
Bug Fixes
ReadableStreamandResponsefunctionality in native-root environments.Tests