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 #2064 · NativeScript/android
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.
Stacked on #2043; the same fix for iOS is NativeScript/ios#490. The new markAsUncloneable spec fails on that branch and passes here, and the full device suite passes. The deserialization race has no deterministic spec.
Summary by CodeRabbit
- Bug Fixes
- Structured cloning now correctly reports a
DataCloneError when it encounters an object marked as uncloneable during serialization.
- Deserialized values with transferables remain single-use, while values without transferables can be read without triggering transferable cleanup.
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:
251f22e8-9341-43d9-9993-1b569a175846
📥 Commits
Reviewing files that changed from the base of the PR and between 42a8bcf and cf36bd1.
📒 Files selected for processing (3)
test-app/app/src/main/assets/app/tests/messaging/uncloneableInGetterWorker.jstest-app/app/src/main/assets/app/tests/testMessaging.jstest-app/runtime/src/main/cpp/StructuredSerialization.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
📝 Walkthrough
Walkthrough
The change updates structured serialization to detect an uncloneable brand created while a getter runs during cloning, and to apply single-consumer handling and transferable-storage cleanup only when transferables are present. It adds a worker regression test for the clone error.
Changes
Structured serialization
| Layer / File(s) |
Summary |
Detect brands created during cloning
test-app/runtime/src/main/cpp/StructuredSerialization.cpp, test-app/app/src/main/assets/app/tests/messaging/uncloneableInGetterWorker.js, test-app/app/src/main/assets/app/tests/testMessaging.js |
IsHostObject refreshes an empty cached uncloneable brand. A worker regression test checks that marking an object uncloneable from a getter causes a DataCloneError. |
Condition cleanup on transferables
test-app/runtime/src/main/cpp/StructuredSerialization.cpp |
Deserialize uses the presence of transferables to control single-consumer marking and cleanup of transferred buffer and port storage. |
Priority: ➖ Normal
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Suggested reviewers: edusperoni
Merge Risk: ⚪ Minimal · up to cf36b
No merge-blocking issue is established; the change is ready to merge after normal checks.
🚥 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 describes both main runtime fixes: read-only clone-only message reads and detection of an uncloneable brand created during cloning. |
| 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
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- 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 watched the getter run,
Then saw the clone report its error.
With transferables, storage clears;
Without them, vectors stay in place.
The rabbit hops through tests with cheer.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Oct 6, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
✅ Action performed
Review finished.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
|
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.
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.
|