diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index c5e1058a3068e1..057d2d062eb57b 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -168,3 +168,4 @@ globals: module: false internalBinding: false primordials: false + JSTransferable: false diff --git a/lib/internal/per_context/domexception.js b/lib/internal/per_context/domexception.js index 5296e54193e497..12b31de568468c 100644 --- a/lib/internal/per_context/domexception.js +++ b/lib/internal/per_context/domexception.js @@ -3,16 +3,24 @@ const { ErrorCaptureStackTrace, ErrorPrototype, + FunctionPrototypeCall, ObjectDefineProperties, ObjectDefineProperty, + ObjectGetOwnPropertyDescriptors, + ObjectGetPrototypeOf, ObjectSetPrototypeOf, - SafeWeakMap, + ReflectConstruct, SafeMap, SafeSet, + SafeWeakMap, + SymbolFor, SymbolToStringTag, TypeError, } = primordials; +const kClone = SymbolFor('nodejs.worker_threads.clone'); +const kDeserialize = SymbolFor('nodejs.worker_threads.deserialize'); + function throwInvalidThisError(Base, type) { const err = new Base(); const key = 'ERR_INVALID_THIS'; @@ -64,14 +72,51 @@ function ensureInitialized() { isInitialized = true; } +// This is the same as https://github.com/nodejs/node/blob/12e3c74e2edf78eeae662dce0a9db17a6fe7d730/lib/internal/worker/js_transferable.js#L41-L46. +// It has been copied here because require() is not available inside this +// context. +function makeTransferable(obj) { + const inst = ReflectConstruct(JSTransferable, [], obj.constructor); + ObjectDefineProperties(inst, ObjectGetOwnPropertyDescriptors(obj)); + ObjectSetPrototypeOf(inst, ObjectGetPrototypeOf(obj)); + return inst; +} + class DOMException { constructor(message = '', name = 'Error') { ensureInitialized(); ErrorCaptureStackTrace(this); - internalsMap.set(this, { - message: `${message}`, - name: `${name}` + const code = nameToCodeMap.get(name) || 0; + // We need to define these properties on this throw away object because + // makeTransferable() indirectly invokes Error#toString(), which calls the + // getters on `this` object but the getters are designed to work only on the + // `internals` object. + ObjectDefineProperties(this, { + message: { + configurable: false, + enumerable: false, + value: message, + writable: false, + }, + name: { + configurable: false, + enumerable: false, + value: name, + writable: false, + }, + code: { + configurable: false, + enumerable: false, + value: code, + writable: false, + } }); + const internals = makeTransferable(this); + FunctionPrototypeCall(this[kDeserialize], + internals, + { message, name, code }); + // eslint-disable-next-line no-constructor-return + return internals; } get name() { @@ -103,17 +148,51 @@ class DOMException { return 0; } - const code = nameToCodeMap.get(internals.name); - return code === undefined ? 0 : code; + return internals.code; + } + + [kClone]() { + const message = this.message; + const name = this.name; + const code = this.code; + return { + data: { message, name, code }, + deserializeInfo: 'internal/per_context/domexception:DOMException', + }; + } + + [kDeserialize]({ message, name, code }) { + ensureInitialized(); + internalsMap.set(this, { message: `${message}`, name: `${name}`, code }); } } ObjectSetPrototypeOf(DOMException.prototype, ErrorPrototype); ObjectDefineProperties(DOMException.prototype, { - [SymbolToStringTag]: { configurable: true, value: 'DOMException' }, - name: { enumerable: true, configurable: true }, - message: { enumerable: true, configurable: true }, - code: { enumerable: true, configurable: true } + [SymbolToStringTag]: { + configurable: true, + enumerable: false, + value: 'DOMException', + writable: false, + }, + name: { + configurable: true, + enumerable: true, + value: undefined, + writable: false, + }, + message: { + configurable: true, + enumerable: true, + value: undefined, + writable: false, + }, + code: { + configurable: true, + enumerable: true, + value: undefined, + writable: false, + } }); function forEachCode(fn) { @@ -147,7 +226,12 @@ function forEachCode(fn) { } forEachCode((name, codeName, value) => { - const desc = { enumerable: true, value }; + const desc = { + configurable: false, + enumerable: true, + value, + writable: false, + }; ObjectDefineProperty(DOMException, codeName, desc); ObjectDefineProperty(DOMException.prototype, codeName, desc); }); diff --git a/lib/internal/webstreams/transfer.js b/lib/internal/webstreams/transfer.js index 985d7e86738f35..39b4e35689d1ac 100644 --- a/lib/internal/webstreams/transfer.js +++ b/lib/internal/webstreams/transfer.js @@ -1,9 +1,7 @@ 'use strict'; const { - ObjectDefineProperties, PromiseResolve, - ReflectConstruct, } = primordials; const { @@ -34,72 +32,6 @@ const { const assert = require('internal/assert'); -const { - makeTransferable, - kClone, - kDeserialize, -} = require('internal/worker/js_transferable'); - -// This class is a bit of a hack. The Node.js implementation of -// DOMException is not transferable/cloneable. This provides us -// with a variant that is. Unfortunately, it means playing around -// a bit with the message, name, and code properties and the -// prototype. We can revisit this if DOMException is ever made -// properly cloneable. -class CloneableDOMException extends DOMException { - constructor(message, name) { - super(message, name); - this[kDeserialize]({ - message: this.message, - name: this.name, - code: this.code, - }); - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); - } - - [kClone]() { - return { - data: { - message: this.message, - name: this.name, - code: this.code, - }, - deserializeInfo: - 'internal/webstreams/transfer:InternalCloneableDOMException' - }; - } - - [kDeserialize]({ message, name, code }) { - ObjectDefineProperties(this, { - message: { - configurable: true, - enumerable: true, - get() { return message; }, - }, - name: { - configurable: true, - enumerable: true, - get() { return name; }, - }, - code: { - configurable: true, - enumerable: true, - get() { return code; }, - }, - }); - } -} - -function InternalCloneableDOMException() { - return makeTransferable( - ReflectConstruct( - CloneableDOMException, - [], - DOMException)); -} -InternalCloneableDOMException[kDeserialize] = () => {}; - class CrossRealmTransformReadableSource { constructor(port) { this[kState] = { @@ -133,7 +65,7 @@ class CrossRealmTransformReadableSource { }; port.onmessageerror = () => { - const error = new CloneableDOMException( + const error = new DOMException( 'Internal transferred ReadableStream error', 'DataCloneError'); port.postMessage({ type: 'error', value: error }); @@ -156,10 +88,6 @@ class CrossRealmTransformReadableSource { try { this[kState].port.postMessage({ type: 'error', value: reason }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); throw error; } finally { @@ -200,7 +128,7 @@ class CrossRealmTransformWritableSink { } }; port.onmessageerror = () => { - const error = new CloneableDOMException( + const error = new DOMException( 'Internal transferred ReadableStream error', 'DataCloneError'); port.postMessage({ type: 'error', value: error }); @@ -229,10 +157,6 @@ class CrossRealmTransformWritableSink { try { this[kState].port.postMessage({ type: 'chunk', value: chunk }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); this[kState].port.close(); throw error; @@ -248,10 +172,6 @@ class CrossRealmTransformWritableSink { try { this[kState].port.postMessage({ type: 'error', value: reason }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); throw error; } finally { @@ -294,6 +214,4 @@ module.exports = { newCrossRealmWritableSink, CrossRealmTransformWritableSink, CrossRealmTransformReadableSource, - CloneableDOMException, - InternalCloneableDOMException, }; diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js index 7bd6c8cafc32e2..464a3e81aa7a81 100644 --- a/lib/internal/worker/js_transferable.js +++ b/lib/internal/worker/js_transferable.js @@ -7,11 +7,10 @@ const { ObjectSetPrototypeOf, ReflectConstruct, StringPrototypeSplit, + SymbolFor, } = primordials; const { - messaging_deserialize_symbol, messaging_transfer_symbol, - messaging_clone_symbol, messaging_transfer_list_symbol } = internalBinding('symbols'); const { @@ -27,7 +26,8 @@ function setup() { const { 0: module, 1: ctor } = StringPrototypeSplit(deserializeInfo, ':'); const Ctor = require(module)[ctor]; if (typeof Ctor !== 'function' || - typeof Ctor.prototype[messaging_deserialize_symbol] !== 'function') { + typeof Ctor.prototype[ + SymbolFor('nodejs.worker_threads.deserialize')] !== 'function') { // Not one of the official errors because one should not be able to get // here without messing with Node.js internals. // eslint-disable-next-line no-restricted-syntax @@ -49,8 +49,8 @@ module.exports = { makeTransferable, setup, JSTransferable, - kClone: messaging_clone_symbol, - kDeserialize: messaging_deserialize_symbol, + kClone: SymbolFor('nodejs.worker_threads.clone'), + kDeserialize: SymbolFor('nodejs.worker_threads.deserialize'), kTransfer: messaging_transfer_symbol, kTransferList: messaging_transfer_list_symbol, }; diff --git a/src/api/environment.cc b/src/api/environment.cc index 1ec1e8bc8c1896..836bb96163310f 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -2,6 +2,7 @@ #include "node_context_data.h" #include "node_errors.h" #include "node_internals.h" +#include "node_messaging.h" #include "node_native_module_env.h" #include "node_platform.h" #include "node_v8_platform-inl.h" @@ -659,6 +660,8 @@ Maybe InitializePrimordials(Local context) { FIXED_ONE_BYTE_STRING(isolate, "primordials"); Local global_string = FIXED_ONE_BYTE_STRING(isolate, "global"); Local exports_string = FIXED_ONE_BYTE_STRING(isolate, "exports"); + Local js_transferable_string = + FIXED_ONE_BYTE_STRING(isolate, "JSTransferable"); // Create primordials first and make it available to per-context scripts. Local primordials = Object::New(isolate); @@ -675,8 +678,11 @@ Maybe InitializePrimordials(Local context) { for (const char** module = context_files; *module != nullptr; module++) { std::vector> parameters = { - global_string, exports_string, primordials_string}; - Local arguments[] = {context->Global(), exports, primordials}; + global_string, exports_string, primordials_string, + js_transferable_string}; + Local arguments[] = { + context->Global(), exports, primordials, + worker::JSTransferable::GetConstructorFunction(isolate)}; MaybeLocal maybe_fn = native_module::NativeModuleEnv::LookupAndCompile( context, *module, ¶meters, nullptr); diff --git a/src/base_object.h b/src/base_object.h index 1c63da92fd80c0..9884caebe4557a 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -112,6 +112,9 @@ class BaseObject : public MemoryRetainer { static v8::Local GetConstructorTemplate( Environment* env); + static v8::Local GetConstructorTemplate( + v8::Isolate* isolate); + // Interface for transferring BaseObject instances using the .postMessage() // method of MessagePorts (and, by extension, Workers). // GetTransferMode() returns a transfer mode that indicates how to deal with diff --git a/src/env-inl.h b/src/env-inl.h index 0a2a70eddd938c..a478ddca5a9dea 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -1261,6 +1261,14 @@ void Environment::set_process_exit_handler( #undef VY #undef VP +#define V(PropertyName, StringValue) \ + inline v8::Local IsolateData::PropertyName() const { \ + return v8::Symbol::For(isolate_, FIXED_ONE_BYTE_STRING(isolate_, \ + StringValue)); \ + } + PER_ISOLATE_PUBLIC_SYMBOL_PROPERTIES(V) +#undef V + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) @@ -1270,6 +1278,7 @@ void Environment::set_process_exit_handler( } PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_SYMBOL_PROPERTIES(VY) + PER_ISOLATE_PUBLIC_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS @@ -1287,6 +1296,16 @@ void Environment::set_process_exit_handler( ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) #undef V +#define V(PropertyName, TypeName) \ + inline v8::Local Environment::PropertyName() const { \ + return get_ ## PropertyName(isolate()); \ + } \ + inline void Environment::set_ ## PropertyName(v8::Local value) { \ + set_ ## PropertyName(isolate(), value); \ + } + PER_ISOLATE_ETERNALS(V) +#undef V + v8::Local Environment::context() const { return PersistentToLocal::Strong(context_); } diff --git a/src/env.cc b/src/env.cc index f2cb5243944c9d..9f2468d05cf2c0 100644 --- a/src/env.cc +++ b/src/env.cc @@ -62,6 +62,22 @@ int const Environment::kNodeContextTag = 0x6e6f64; void* const Environment::kNodeContextTagPtr = const_cast( static_cast(&Environment::kNodeContextTag)); +#define V(PropertyName, TypeName) \ + static v8::Eternal PropertyName ## _eternal; + PER_ISOLATE_ETERNALS(V) +#undef V + +#define V(PropertyName, TypeName) \ + Local Environment::get_ ## PropertyName(Isolate* isolate) { \ + return PropertyName ## _eternal.Get(isolate); \ + } \ + void Environment::set_ ## PropertyName(Isolate* isolate, \ + Local value) { \ + PropertyName ## _eternal.Set(isolate, value); \ + } + PER_ISOLATE_ETERNALS(V) +#undef V + std::vector IsolateData::Serialize(SnapshotCreator* creator) { Isolate* isolate = creator->GetIsolate(); std::vector indexes; @@ -1354,6 +1370,7 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) { id++; \ } while (0); ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) + PER_ISOLATE_ETERNALS(V) #undef V id = 0; @@ -1488,6 +1505,7 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) { #define V(PropertyName, TypeName) SetProperty(PropertyName, TypeName, \ templates, template, isolate_) ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V); + PER_ISOLATE_ETERNALS(V) #undef V i = 0; // index to the array @@ -1736,11 +1754,23 @@ bool BaseObject::IsRootNode() const { } Local BaseObject::GetConstructorTemplate(Environment* env) { - Local tmpl = env->base_object_ctor_template(); + return GetConstructorTemplate(env->isolate()); +} + +Local BaseObject::GetConstructorTemplate(Isolate* isolate) { + Local tmpl = + Environment::get_base_object_ctor_template(isolate); if (tmpl.IsEmpty()) { - tmpl = env->NewFunctionTemplate(nullptr); - tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "BaseObject")); - env->set_base_object_ctor_template(tmpl); + tmpl = FunctionTemplate::New(isolate, + nullptr, + Local(), + Local(), + 0, + v8::ConstructorBehavior::kAllow, + v8::SideEffectType::kHasSideEffect, + nullptr); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "BaseObject")); + Environment::set_base_object_ctor_template(isolate, tmpl); } return tmpl; } diff --git a/src/env.h b/src/env.h index 8689129041618b..b62670b8ce1e98 100644 --- a/src/env.h +++ b/src/env.h @@ -167,9 +167,7 @@ constexpr size_t kFsStatsBufferLength = V(async_id_symbol, "async_id_symbol") \ V(handle_onclose_symbol, "handle_onclose") \ V(no_message_symbol, "no_message_symbol") \ - V(messaging_deserialize_symbol, "messaging_deserialize_symbol") \ V(messaging_transfer_symbol, "messaging_transfer_symbol") \ - V(messaging_clone_symbol, "messaging_clone_symbol") \ V(messaging_transfer_list_symbol, "messaging_transfer_list_symbol") \ V(oninit_symbol, "oninit") \ V(owner_symbol, "owner_symbol") \ @@ -177,6 +175,11 @@ constexpr size_t kFsStatsBufferLength = V(resource_symbol, "resource_symbol") \ V(trigger_async_id_symbol, "trigger_async_id_symbol") \ +#define PER_ISOLATE_PUBLIC_SYMBOL_PROPERTIES(V) \ + V(nodejs_worker_threads_clone_symbol, "nodejs.worker_threads.clone") \ + V(nodejs_worker_threads_deserialize_symbol, \ + "nodejs.worker_threads.deserialize") \ + // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. #define PER_ISOLATE_STRING_PROPERTIES(V) \ @@ -456,10 +459,12 @@ constexpr size_t kFsStatsBufferLength = V(x_forwarded_string, "x-forwarded-for") \ V(zero_return_string, "ZERO_RETURN") +#define PER_ISOLATE_ETERNALS(V) \ + V(base_object_ctor_template, v8::FunctionTemplate) + #define ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) \ V(async_wrap_ctor_template, v8::FunctionTemplate) \ V(async_wrap_object_ctor_template, v8::FunctionTemplate) \ - V(base_object_ctor_template, v8::FunctionTemplate) \ V(binding_data_ctor_template, v8::FunctionTemplate) \ V(blob_constructor_template, v8::FunctionTemplate) \ V(blocklist_constructor_template, v8::FunctionTemplate) \ @@ -586,6 +591,7 @@ class IsolateData : public MemoryRetainer { inline v8::Local PropertyName() const; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_SYMBOL_PROPERTIES(VY) + PER_ISOLATE_PUBLIC_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VY @@ -1315,6 +1321,7 @@ class Environment : public MemoryRetainer { inline v8::Local PropertyName() const; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_SYMBOL_PROPERTIES(VY) + PER_ISOLATE_PUBLIC_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS @@ -1326,6 +1333,14 @@ class Environment : public MemoryRetainer { inline void set_ ## PropertyName(v8::Local value); ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) + PER_ISOLATE_ETERNALS(V) +#undef V + +#define V(PropertyName, TypeName) \ + static v8::Local get_ ## PropertyName(v8::Isolate* isolate); \ + static void set_ ## PropertyName(v8::Isolate* isolate, \ + v8::Local value); + PER_ISOLATE_ETERNALS(V) #undef V inline v8::Local context() const; diff --git a/src/node_messaging.cc b/src/node_messaging.cc index aac1245f269a87..98707b8a59e1ed 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1136,6 +1136,21 @@ void JSTransferable::New(const FunctionCallbackInfo& args) { new JSTransferable(Environment::GetCurrent(args), args.This()); } +Local JSTransferable::GetConstructorFunction(Isolate* isolate) { + Local tmpl = FunctionTemplate::New( + isolate, + New, + Local(), + Local(), + 0, + v8::ConstructorBehavior::kAllow, + v8::SideEffectType::kHasSideEffect, + nullptr); + tmpl->Inherit(BaseObject::GetConstructorTemplate(isolate)); + tmpl->InstanceTemplate()->SetInternalFieldCount(kInternalFieldCount); + return tmpl->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); +} + JSTransferable::TransferMode JSTransferable::GetTransferMode() const { // Implement `kClone in this ? kCloneable : kTransferable`. HandleScope handle_scope(env()->isolate()); @@ -1143,7 +1158,7 @@ JSTransferable::TransferMode JSTransferable::GetTransferMode() const { bool has_clone; if (!object()->Has(env()->context(), - env()->messaging_clone_symbol()).To(&has_clone)) { + env()->nodejs_worker_threads_clone_symbol()).To(&has_clone)) { return TransferMode::kUntransferable; } @@ -1167,7 +1182,8 @@ std::unique_ptr JSTransferable::TransferOrClone( HandleScope handle_scope(env()->isolate()); Local context = env()->isolate()->GetCurrentContext(); Local method_name = mode == TransferMode::kCloneable ? - env()->messaging_clone_symbol() : env()->messaging_transfer_symbol(); + env()->nodejs_worker_threads_clone_symbol() : + env()->messaging_transfer_symbol(); Local method; if (!object()->Get(context, method_name).ToLocal(&method)) { @@ -1242,7 +1258,7 @@ Maybe JSTransferable::FinalizeTransferRead( Local data; if (!deserializer->ReadValue(context).ToLocal(&data)) return Nothing(); - Local method_name = env()->messaging_deserialize_symbol(); + Local method_name = env()->nodejs_worker_threads_deserialize_symbol(); Local method; if (!object()->Get(context, method_name).ToLocal(&method)) { return Nothing(); @@ -1459,13 +1475,10 @@ static void InitMessaging(Local target, env->NewFunctionTemplate(MessageChannel)); } - { - Local t = env->NewFunctionTemplate(JSTransferable::New); - t->Inherit(BaseObject::GetConstructorTemplate(env)); - t->InstanceTemplate()->SetInternalFieldCount( - JSTransferable::kInternalFieldCount); - env->SetConstructorFunction(target, "JSTransferable", t); - } + target->Set( + env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "JSTransferable"), + JSTransferable::GetConstructorFunction(env->isolate())).Check(); env->SetConstructorFunction( target, diff --git a/src/node_messaging.h b/src/node_messaging.h index 643604e4a99f15..2121e6e0679df4 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -322,6 +322,7 @@ class JSTransferable : public BaseObject { public: JSTransferable(Environment* env, v8::Local obj); static void New(const v8::FunctionCallbackInfo& args); + static v8::Local GetConstructorFunction(v8::Isolate* isolate); TransferMode GetTransferMode() const override; std::unique_ptr TransferForMessaging() override; diff --git a/test/parallel/test-errors-cloneabledomexception.js b/test/parallel/test-errors-cloneabledomexception.js new file mode 100644 index 00000000000000..22b73271adf643 --- /dev/null +++ b/test/parallel/test-errors-cloneabledomexception.js @@ -0,0 +1,24 @@ +'use strict'; + +const common = require('../common'); + +const { + ok, + strictEqual, +} = require('assert'); + +const exception = new DOMException('foo', 'AbortError'); +strictEqual(exception.name, 'AbortError'); +strictEqual(exception.message, 'foo'); +strictEqual(exception.code, 20); + +const mc = new MessageChannel(); +mc.port1.onmessage = common.mustCall(({ data }) => { + ok(data instanceof DOMException); + strictEqual(data.name, 'AbortError'); + strictEqual(data.message, 'foo'); + strictEqual(data.code, 20); + strictEqual(data.stack, exception.stack); + mc.port1.close(); +}); +mc.port2.postMessage(exception);