GeezerVison is showing github.com — fix(runtime): keep clone-only message reads read-only, and see an uncloneable brand created mid-clone by adrian-niculescu · Pull Request #490 · NativeScript/ios
Conversation
adrian-niculescu
commented
Oct 6, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
An object marked with markAsUncloneable from a getter, during the clone that reaches it, is cloned instead of throwing DataCloneError when that is the first markAsUncloneable call in the isolate. The serializer reads the brand once, when it is constructed, and at that point the brand does not exist yet. It now picks the brand up once markAsUncloneable creates it.
Separately, SerializedValue::Deserialize cleared its transfer vectors on every read. A clone-only message is read by every BroadcastChannel receiver, and every getEnvironmentData call, with no lock, so those clears were concurrent writes to shared state, against what the comment on the consumed_ flag intends. Only the single receiver of a message with transferables writes them now.
The same fix for Android is NativeScript/android#2064. The new markAsUncloneable spec fails on main and passes here, and the full TestRunner suite passes. The deserialization race has no deterministic spec.
Summary by CodeRabbit
- Bug Fixes
- Structured cloning now correctly rejects objects marked as uncloneable while their containing object is being serialized, reporting a
DataCloneError.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…loneable brand created mid-clone
A clone-only message is read concurrently by every BroadcastChannel receiver and every getEnvironmentData call, and deserializing it cleared its transfer vectors, a write. Only the single receiver of a message with transferables writes them now.
The serializer cached the uncloneable brand when it was created, so the first markAsUncloneable call in an isolate, made by a getter in the graph being written, went unseen and the marked object was cloned. It now picks the brand up once it exists.
coderabbitai
Bot
commented
Oct 6, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
34ace9bd-7208-41aa-9cb7-5f7db5a960a4
📥 Commits
Reviewing files that changed from the base of the PR and between 4ae32eb and 7d20c28.
📒 Files selected for processing (3)
NativeScript/runtime/StructuredSerialization.cppTestRunner/app/tests/MessagingTests.jsTestRunner/app/tests/messaging/uncloneableInGetterWorker.js
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
📝 Walkthrough
Walkthrough
Structured serialization now checks an uncloneable brand on demand during graph writing. Deserialization uses transferables state to control consumed-state handling and transferred-buffer and transferred-port cleanup. A worker regression test covers an object marked uncloneable by a getter during cloning.
Changes
Uncloneable objects during cloning
| Layer / File(s) |
Summary |
Brand check and worker regression
NativeScript/runtime/StructuredSerialization.cpp, TestRunner/app/tests/messaging/uncloneableInGetterWorker.js, TestRunner/app/tests/MessagingTests.js |
IsHostObject resolves the uncloneable brand when its cached handle is empty. The worker regression test marks a getter-returned object uncloneable during cloning and checks for a DataCloneError. |
Transferable cleanup during deserialization
| Layer / File(s) |
Summary |
Gate deserialization cleanup on transferables
NativeScript/runtime/StructuredSerialization.cpp |
Deserialize stores HasTransferables() in singleReceiver. It marks the message consumed and clears transferred-buffer and transferred-port storage only on that path. |
Priority: ➖ Normal
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Suggested reviewers: edusperoni
Merge Risk: ⚪ Minimal · up to 7d20c
The change makes clone-only message reads read-only and detects an object marked uncloneable during cloning. No actionable merge risk was found.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
⚠️ Warning |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 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 describes both main changes: detecting an uncloneable brand created during cloning and keeping clone-only message reads read-only. |
| 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
- 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
A rabbit checks each cloned leaf,
A getter marks one, beyond belief.
The brand is found as graphs unfold,
A clone error is then told.
Transfer bundles clear when their path says so.
Comment @coderabbitai help to get the list of available commands.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Nov 5, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|