diff --git a/src/env.h b/src/env.h index 1ae707428230e8..8b690c09067135 100644 --- a/src/env.h +++ b/src/env.h @@ -151,6 +151,7 @@ constexpr size_t kFsStatsBufferLength = // "node:" prefix to avoid name clashes with third-party code. #define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \ V(alpn_buffer_private_symbol, "node:alpnBuffer") \ + V(arraybuffer_untransferable_private_symbol, "node:untransferableBuffer") \ V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(contextify_global_private_symbol, "node:contextify:global") \ diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index f29adeb8f8db61..b90db8f3e8ce86 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2614,6 +2614,8 @@ napi_status napi_create_external_arraybuffer(napi_env env, v8::Isolate* isolate = env->isolate; v8::Local buffer = v8::ArrayBuffer::New(isolate, external_data, byte_length); + v8::Maybe marked = env->mark_arraybuffer_as_untransferable(buffer); + CHECK_MAYBE_NOTHING(env, marked, napi_generic_failure); if (finalize_cb != nullptr) { // Create a self-deleting weak reference that invokes the finalizer diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 8eed25bc9e6f77..0d5424cccaddf0 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -39,6 +39,10 @@ struct napi_env__ { inline void Unref() { if ( --refs == 0) delete this; } virtual bool can_call_into_js() const { return true; } + virtual v8::Maybe mark_arraybuffer_as_untransferable( + v8::Local ab) const { + return v8::Just(true); + } template void CallIntoModule(T&& call, U&& handle_exception) { diff --git a/src/node_api.cc b/src/node_api.cc index 20b591747de2d0..0f7981cd5605ae 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -24,6 +24,14 @@ struct node_napi_env__ : public napi_env__ { bool can_call_into_js() const override { return node_env()->can_call_into_js(); } + + v8::Maybe mark_arraybuffer_as_untransferable( + v8::Local ab) const { + return ab->SetPrivate( + context(), + node_env()->arraybuffer_untransferable_private_symbol(), + v8::True(isolate)); + } }; typedef node_napi_env__* node_napi_env; diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 0cbc441c3cd34a..e80604e2c06bd8 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -364,10 +364,8 @@ MaybeLocal New(Isolate* isolate, THROW_ERR_BUFFER_CONTEXT_NOT_AVAILABLE(isolate); return MaybeLocal(); } - Local obj; - if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj)) - return handle_scope.Escape(obj); - return Local(); + return handle_scope.EscapeMaybe( + Buffer::New(env, data, length, callback, hint)); } @@ -385,6 +383,12 @@ MaybeLocal New(Environment* env, } Local ab = ArrayBuffer::New(env->isolate(), data, length); + if (ab->SetPrivate(env->context(), + env->arraybuffer_untransferable_private_symbol(), + True(env->isolate())).IsNothing()) { + callback(data, hint); + return Local(); + } MaybeLocal ui = Buffer::New(env, ab, 0, length); CallbackInfo::New(env->isolate(), ab, callback, data, hint); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 19065fdb7d1be5..b0e7d15a8c40a6 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -329,6 +329,16 @@ Maybe Message::Serialize(Environment* env, !env->isolate_data()->uses_node_allocator()) { continue; } + // See https://github.com/nodejs/node/pull/30339#issuecomment-552225353 + // for details. + bool untransferrable; + if (!ab->HasPrivate( + context, + env->arraybuffer_untransferable_private_symbol()) + .To(&untransferrable)) { + return Nothing(); + } + if (untransferrable) continue; if (std::find(array_buffers.begin(), array_buffers.end(), ab) != array_buffers.end()) { ThrowDataCloneException( diff --git a/test/addons/worker-buffer-callback/binding.cc b/test/addons/worker-buffer-callback/binding.cc new file mode 100644 index 00000000000000..a40876ebb523a6 --- /dev/null +++ b/test/addons/worker-buffer-callback/binding.cc @@ -0,0 +1,29 @@ +#include +#include +#include + +using v8::Context; +using v8::Isolate; +using v8::Local; +using v8::Object; +using v8::Value; + +char data[] = "hello"; + +void Initialize(Local exports, + Local module, + Local context) { + Isolate* isolate = context->GetIsolate(); + exports->Set(context, + v8::String::NewFromUtf8( + isolate, "buffer", v8::NewStringType::kNormal) + .ToLocalChecked(), + node::Buffer::New( + isolate, + data, + sizeof(data), + [](char* data, void* hint) {}, + nullptr).ToLocalChecked()).Check(); +} + +NODE_MODULE_CONTEXT_AWARE(NODE_GYP_MODULE_NAME, Initialize) diff --git a/test/addons/worker-buffer-callback/binding.gyp b/test/addons/worker-buffer-callback/binding.gyp new file mode 100644 index 00000000000000..55fbe7050f18e4 --- /dev/null +++ b/test/addons/worker-buffer-callback/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ], + 'includes': ['../common.gypi'], + } + ] +} diff --git a/test/addons/worker-buffer-callback/test.js b/test/addons/worker-buffer-callback/test.js new file mode 100644 index 00000000000000..b04984f1576432 --- /dev/null +++ b/test/addons/worker-buffer-callback/test.js @@ -0,0 +1,15 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const { MessageChannel } = require('worker_threads'); +const { buffer } = require(`./build/${common.buildType}/binding`); + +// Test that buffers allocated with a free callback through our APIs are not +// transfered. + +const { port1 } = new MessageChannel(); +const origByteLength = buffer.byteLength; +port1.postMessage(buffer, [buffer.buffer]); + +assert.strictEqual(buffer.byteLength, origByteLength); +assert.notStrictEqual(buffer.byteLength, 0);