From e66a2acc4cb9fc09fc32d1833b89ae56468a0931 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 Nov 2019 20:40:46 +0200 Subject: src: migrate off ArrayBuffer::GetContents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V8 deprecates `GetContents()` in favour of `GetBackingStore()`. Update our code to reflect that. V8 also deprecates `Externalize()` and `IsExternal()`; we should be able to remove all usage of this once V8 8.0 is there. PR-URL: https://github.com/nodejs/node/pull/30339 Refs: https://github.com/v8/v8/commit/bfe3d6bce734e596e312465e207bcfd55a59fe34 Reviewed-By: Gus Caplan Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig Reviewed-By: Jiawen Geng Reviewed-By: David Carlier --- src/node_messaging.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'src/node_messaging.cc') diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 6645ca025b..c54958a4cf 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -298,12 +298,19 @@ Maybe Message::Serialize(Environment* env, // Currently, we support ArrayBuffers and MessagePorts. if (entry->IsArrayBuffer()) { Local ab = entry.As(); - // If we cannot render the ArrayBuffer unusable in this Isolate and - // take ownership of its memory, copying the buffer will have to do. - if (!ab->IsDetachable() || ab->IsExternal() || - !env->isolate_data()->uses_node_allocator()) { + // If we cannot render the ArrayBuffer unusable in this Isolate, + // copying the buffer will have to do. + // Note that we can currently transfer ArrayBuffers even if they were + // not allocated by Node’s ArrayBufferAllocator in the first place, + // because we pass the underlying v8::BackingStore around rather than + // raw data *and* an Isolate with a non-default ArrayBuffer allocator + // is always going to outlive any Workers it creates, and so will its + // allocator along with it. + // TODO(addaleax): Eventually remove the IsExternal() condition, + // see https://github.com/nodejs/node/pull/30339#issuecomment-552225353 + // for details. + if (!ab->IsDetachable() || ab->IsExternal()) continue; - } if (std::find(array_buffers.begin(), array_buffers.end(), ab) != array_buffers.end()) { ThrowDataCloneException( @@ -363,7 +370,9 @@ Maybe Message::Serialize(Environment* env, for (Local ab : array_buffers) { // If serialization succeeded, we render it inaccessible in this Isolate. std::shared_ptr backing_store = ab->GetBackingStore(); - ab->Externalize(backing_store); + // TODO(addaleax): This can/should be dropped once we have V8 8.0. + if (!ab->IsExternal()) + ab->Externalize(backing_store); ab->Detach(); array_buffers_.emplace_back(std::move(backing_store)); -- cgit v1.2.3