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);