Skip to content

Commit

Permalink
src,lib: make DOMException cloneable
Browse files Browse the repository at this point in the history 
This attepts to make DOMException cloneable by making the JSTransferable
constructor and the required symbols available to the per_context scripts.

Refs: nodejs#41038
Signed-off-by: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
@RaisinTen
RaisinTen committed Jan 26, 2022
1 parent b2edcfe commit 19c3b4a
Show file tree
Hide file tree
Showing 12 changed files with 234 additions and 119 deletions.
1 change: 1 addition & 0 deletions lib/.eslintrc.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,4 @@ globals:
module: false
internalBinding: false
primordials: false
JSTransferable: false
107 changes: 96 additions & 11 deletions lib/internal/per_context/domexception.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,23 @@

const {
Error,
FunctionPrototypeCall,
ObjectDefineProperties,
ObjectDefineProperty,
SafeWeakMap,
ObjectGetOwnPropertyDescriptors,
ObjectGetPrototypeOf,
ObjectSetPrototypeOf,
ReflectConstruct,
SafeMap,
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';
Expand Down Expand Up @@ -52,14 +61,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 extends Error {
constructor(message = '', name = 'Error') {
ensureInitialized();
super();
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() {
Expand All @@ -86,16 +132,50 @@ class DOMException extends Error {
if (internals === undefined) {
throwInvalidThisError(TypeError, 'DOMException');
}
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 });
}
}

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) {
Expand Down Expand Up @@ -129,7 +209,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);
});
Expand Down
86 changes: 2 additions & 84 deletions lib/internal/webstreams/transfer.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
'use strict';

const {
ObjectDefineProperties,
PromiseResolve,
ReflectConstruct,
} = primordials;

const {
Expand Down Expand Up @@ -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] = {
Expand Down Expand Up @@ -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 });
Expand All @@ -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 {
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -294,6 +214,4 @@ module.exports = {
newCrossRealmWritableSink,
CrossRealmTransformWritableSink,
CrossRealmTransformReadableSource,
CloneableDOMException,
InternalCloneableDOMException,
};
10 changes: 5 additions & 5 deletions lib/internal/worker/js_transferable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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,
};
10 changes: 8 additions & 2 deletions src/api/environment.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -659,6 +660,8 @@ Maybe<bool> InitializePrimordials(Local<Context> context) {
FIXED_ONE_BYTE_STRING(isolate, "primordials");
Local<String> global_string = FIXED_ONE_BYTE_STRING(isolate, "global");
Local<String> exports_string = FIXED_ONE_BYTE_STRING(isolate, "exports");
Local<String> js_transferable_string =
FIXED_ONE_BYTE_STRING(isolate, "JSTransferable");

// Create primordials first and make it available to per-context scripts.
Local<Object> primordials = Object::New(isolate);
Expand All @@ -675,8 +678,11 @@ Maybe<bool> InitializePrimordials(Local<Context> context) {

for (const char** module = context_files; *module != nullptr; module++) {
std::vector<Local<String>> parameters = {
global_string, exports_string, primordials_string};
Local<Value> arguments[] = {context->Global(), exports, primordials};
global_string, exports_string, primordials_string,
js_transferable_string};
Local<Value> arguments[] = {
context->Global(), exports, primordials,
worker::JSTransferable::GetConstructorFunction(isolate)};
MaybeLocal<Function> maybe_fn =
native_module::NativeModuleEnv::LookupAndCompile(
context, *module, &parameters, nullptr);
Expand Down
3 changes: 3 additions & 0 deletions src/base_object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ class BaseObject : public MemoryRetainer {
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
Environment* env);

static v8::Local<v8::FunctionTemplate> 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
Expand Down
Loading

0 comments on commit 19c3b4a

Please sign in to comment.