From 7d20c283907edbc135fadd40a6856cb805dfc3a3 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:32:14 +0300 Subject: [PATCH] fix(runtime): keep clone-only message reads read-only, and see an uncloneable 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. --- .../runtime/StructuredSerialization.cpp | 17 +++++++++++++---- TestRunner/app/tests/MessagingTests.js | 18 ++++++++++++++++++ .../messaging/uncloneableInGetterWorker.js | 19 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 TestRunner/app/tests/messaging/uncloneableInGetterWorker.js diff --git a/NativeScript/runtime/StructuredSerialization.cpp b/NativeScript/runtime/StructuredSerialization.cpp index 789d9430..f08f76cb 100644 --- a/NativeScript/runtime/StructuredSerialization.cpp +++ b/NativeScript/runtime/StructuredSerialization.cpp @@ -161,6 +161,11 @@ class SerializerDelegate : public ValueSerializer::Delegate { if (object->InternalFieldCount() > 0) { return Just(true); } + if (uncloneableBrand_.IsEmpty()) { + // markAsUncloneable creates the brand on its first call in an isolate, + // which a getter in the graph being written can make. + uncloneableBrand_ = messaging::UncloneableBrandIfAny(isolate); + } if (!uncloneableBrand_.IsEmpty()) { bool uncloneable = false; if (!object->HasPrivate(isolate->GetCurrentContext(), uncloneableBrand_) @@ -622,7 +627,8 @@ MaybeLocal SerializedValue::Deserialize(Isolate* isolate, "A message carrying transferred objects can only be read once."); return MaybeLocal(); } - if (HasTransferables()) { + const bool singleReceiver = HasTransferables(); + if (singleReceiver) { consumed_ = true; } @@ -716,9 +722,12 @@ MaybeLocal SerializedValue::Deserialize(Isolate* isolate, ArrayBuffer::New(isolate, std::move(transferredBuffers_[i]))); } // Handed over above; the vectors would otherwise keep reporting - // transferables that are no longer here. - transferredBuffers_.clear(); - transferredPorts_.clear(); + // transferables that are no longer here. Only the single receiver writes + // them: the other readers share this value with no lock. + if (singleReceiver) { + transferredBuffers_.clear(); + transferredPorts_.clear(); + } Local result; { diff --git a/TestRunner/app/tests/MessagingTests.js b/TestRunner/app/tests/MessagingTests.js index f6695984..e9acebc9 100644 --- a/TestRunner/app/tests/MessagingTests.js +++ b/TestRunner/app/tests/MessagingTests.js @@ -295,6 +295,24 @@ describe("Messaging runtime edges", function () { }); }); + describe("markAsUncloneable", function () { + it("rejects an object marked while the clone that reaches it is written", function (done) { + var worker = new Worker("./messaging/uncloneableInGetterWorker.js"); + worker.onmessage = function (event) { + expect(event.data).toEqual({ threw: true, name: "DataCloneError" }); + worker.terminate(); + done(); + }; + // fail() throws in this runner, which would skip done() when called + // from an event handler. + worker.onerror = function (error) { + expect("worker error: " + error.message).toBeNull(); + worker.terminate(); + done(); + }; + }); + }); + describe("worker error reporting", function () { // A worker boots on its own thread, so the first error arrives whenever // the runner gets to it; specs wait for it and only then settle for diff --git a/TestRunner/app/tests/messaging/uncloneableInGetterWorker.js b/TestRunner/app/tests/messaging/uncloneableInGetterWorker.js new file mode 100644 index 00000000..f7101ab3 --- /dev/null +++ b/TestRunner/app/tests/messaging/uncloneableInGetterWorker.js @@ -0,0 +1,19 @@ +// A fresh isolate: the markAsUncloneable call in the getter below is the first +// one this isolate has seen, and it happens while the clone that reaches the +// marked object is already being written. +var markAsUncloneable = require("node:worker_threads").markAsUncloneable; +var graph = { + get inner() { + var marked = { a: 1 }; + markAsUncloneable(marked); + return marked; + }, +}; +var result; +try { + structuredClone(graph); + result = { threw: false }; +} catch (e) { + result = { threw: true, name: e && e.name }; +} +postMessage(result);