src: add receiver to fast api callback methods

When creating an fast api the callback might use the receiver. In that
case if the internal binding is destructured the method won't have
access to the reciver and it will throw. Passing the receiver as second
argument ensures the receiver is available.

PR-URL: https://github.com/nodejs/node/pull/54408
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This commit is contained in:
Carlos Espa 2024-09-28 11:46:03 +02:00 committed by GitHub
parent 18acff0d01
commit f5d454ac7e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 123 additions and 53 deletions

View File

@ -220,7 +220,7 @@ The initializer module also needs to be allowed. Consider the following example:
```console
$ node --experimental-permission t.js
node:internal/modules/cjs/loader:162
const result = internalModuleStat(filename);
const result = internalModuleStat(receiver, filename);
^
Error: Access to this API has been restricted

View File

@ -48,7 +48,7 @@ will be restricted.
```console
$ node --experimental-permission index.js
node:internal/modules/cjs/loader:171
const result = internalModuleStat(filename);
const result = internalModuleStat(receiver, filename);
^
Error: Access to this API has been restricted

View File

@ -24,7 +24,7 @@ for example, they may not trigger garbage collection.
[`node_external_reference.h`](../../src/node_external_reference.h) file.
Although, it would not start failing or crashing until the function ends up
in a snapshot (either the built-in or a user-land one). Please refer to the
[binding functions documentation](../../src#binding-functions) for more
[binding functions documentation](../../src/README.md#binding-functions) for more
information.
* To test fast APIs, make sure to run the tests in a loop with a decent
iterations count to trigger relevant optimizations that prefer the fast API
@ -38,6 +38,23 @@ for example, they may not trigger garbage collection.
* The fast callback must be idempotent up to the point where error and fallback
conditions are checked, because otherwise executing the slow callback might
produce visible side effects twice.
* If the receiver is used in the callback, it must be passed as a second argument,
leaving the first one unused, to prevent the JS land from accidentally omitting the receiver when
invoking the fast API method.
```cpp
// Instead of invoking the method as `receiver.internalModuleStat(input)`, the JS land should
// invoke it as `internalModuleStat(binding, input)` to make sure the binding is available to
// the native land.
static int32_t FastInternalModuleStat(
Local<Object> unused,
Local<Object> recv,
const FastOneByteString& input,
FastApiCallbackOptions& options) {
Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
// More code
}
```
## Fallback to slow path

View File

@ -1411,7 +1411,7 @@ function readdirSyncRecursive(basePath, options) {
for (let i = 0; i < readdirResult.length; i++) {
const resultPath = pathModule.join(path, readdirResult[i]);
const relativeResultPath = pathModule.relative(basePath, resultPath);
const stat = binding.internalModuleStat(resultPath);
const stat = binding.internalModuleStat(binding, resultPath);
ArrayPrototypePush(readdirResults, relativeResultPath);
// 1 indicates directory
if (stat === 1) {

View File

@ -915,7 +915,7 @@ async function readdirRecursive(originalPath, options) {
const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
for (const ent of readdir) {
const direntPath = pathModule.join(path, ent);
const stat = binding.internalModuleStat(direntPath);
const stat = binding.internalModuleStat(binding, direntPath);
ArrayPrototypePush(
result,
pathModule.relative(originalPath, direntPath),

View File

@ -235,9 +235,9 @@ function stat(filename) {
const result = statCache.get(filename);
if (result !== undefined) { return result; }
}
const result = internalFsBinding.internalModuleStat(filename);
const result = internalFsBinding.internalModuleStat(internalFsBinding, filename);
if (statCache !== null && result >= 0) {
// Only set cache when `internalModuleStat(filename)` succeeds.
// Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
statCache.set(filename, result);
}
return result;

View File

@ -241,8 +241,10 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
throw err;
}
const stats = internalFsBinding.internalModuleStat(StringPrototypeEndsWith(path, '/') ?
StringPrototypeSlice(path, -1) : path);
const stats = internalFsBinding.internalModuleStat(
internalFsBinding,
StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path,
);
// Check for stats.isDirectory()
if (stats === 1) {
@ -802,6 +804,7 @@ function packageResolve(specifier, base, conditions) {
let lastPath;
do {
const stat = internalFsBinding.internalModuleStat(
internalFsBinding,
StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),
);
// Check for !stat.isDirectory()

View File

@ -170,7 +170,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
(*histogram)->RecordDelta();
}
void HistogramBase::FastRecordDelta(Local<Value> receiver) {
void HistogramBase::FastRecordDelta(Local<Value> unused,
Local<Value> receiver) {
HistogramBase* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
(*histogram)->RecordDelta();
@ -190,7 +191,8 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
(*histogram)->Record(value);
}
void HistogramBase::FastRecord(Local<Value> receiver,
void HistogramBase::FastRecord(Local<Value> unused,
Local<Value> receiver,
const int64_t value,
FastApiCallbackOptions& options) {
if (value < 1) {
@ -438,7 +440,9 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
}
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
void IntervalHistogram::FastStart(Local<Value> unused,
Local<Value> receiver,
bool reset) {
IntervalHistogram* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
@ -450,7 +454,7 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
histogram->OnStop();
}
void IntervalHistogram::FastStop(Local<Value> receiver) {
void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
IntervalHistogram* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
histogram->OnStop();
@ -566,42 +570,45 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
(*histogram)->Reset();
}
void HistogramImpl::FastReset(Local<Value> receiver) {
void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
(*histogram)->Reset();
}
double HistogramImpl::FastGetCount(Local<Value> receiver) {
double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Count());
}
double HistogramImpl::FastGetMin(Local<Value> receiver) {
double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Min());
}
double HistogramImpl::FastGetMax(Local<Value> receiver) {
double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Max());
}
double HistogramImpl::FastGetMean(Local<Value> receiver) {
double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return (*histogram)->Mean();
}
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
double HistogramImpl::FastGetExceeds(Local<Value> unused,
Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Exceeds());
}
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
double HistogramImpl::FastGetStddev(Local<Value> unused,
Local<Value> receiver) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return (*histogram)->Stddev();
}
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
double HistogramImpl::FastGetPercentile(Local<Value> unused,
Local<Value> receiver,
const double percentile) {
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Percentile(percentile));

View File

@ -101,14 +101,22 @@ class HistogramImpl {
static void GetPercentilesBigInt(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastReset(v8::Local<v8::Value> receiver);
static double FastGetCount(v8::Local<v8::Value> receiver);
static double FastGetMin(v8::Local<v8::Value> receiver);
static double FastGetMax(v8::Local<v8::Value> receiver);
static double FastGetMean(v8::Local<v8::Value> receiver);
static double FastGetExceeds(v8::Local<v8::Value> receiver);
static double FastGetStddev(v8::Local<v8::Value> receiver);
static double FastGetPercentile(v8::Local<v8::Value> receiver,
static void FastReset(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetCount(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMin(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMax(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMean(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetExceeds(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetStddev(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetPercentile(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
const double percentile);
static void AddMethods(v8::Isolate* isolate,
@ -158,10 +166,12 @@ class HistogramBase final : public BaseObject, public HistogramImpl {
static void Add(const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastRecord(
v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
const int64_t value,
v8::FastApiCallbackOptions& options); // NOLINT(runtime/references)
static void FastRecordDelta(v8::Local<v8::Value> receiver);
static void FastRecordDelta(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
HistogramBase(
Environment* env,
@ -233,8 +243,11 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl {
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Stop(const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
static void FastStop(v8::Local<v8::Value> receiver);
static void FastStart(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
bool reset);
static void FastStop(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
BaseObject::TransferMode GetTransferMode() const override {
return TransferMode::kCloneable;

View File

@ -12,19 +12,25 @@ namespace node {
using CFunctionCallbackWithOneByteString =
uint32_t (*)(v8::Local<v8::Value>, const v8::FastOneByteString&);
using CFunctionCallback = void (*)(v8::Local<v8::Value> receiver);
using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
using CFunctionCallbackReturnDouble =
double (*)(v8::Local<v8::Object> receiver);
double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);
using CFunctionCallbackReturnInt32 =
int32_t (*)(v8::Local<v8::Object> receiver,
int32_t (*)(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
const v8::FastOneByteString& input,
// NOLINTNEXTLINE(runtime/references) This is V8 api.
v8::FastApiCallbackOptions& options);
using CFunctionCallbackValueReturnDouble =
double (*)(v8::Local<v8::Value> receiver);
using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> receiver,
using CFunctionCallbackValueReturnDoubleUnusedReceiver =
double (*)(v8::Local<v8::Value> unused, v8::Local<v8::Value> receiver);
using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
int64_t);
using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> receiver,
using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
bool);
using CFunctionCallbackWithString =
bool (*)(v8::Local<v8::Value>, const v8::FastOneByteString& input);
@ -50,11 +56,15 @@ using CFunctionCallbackWithUint8ArrayUint32Int64Bool =
using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
const uint32_t input);
using CFunctionWithDoubleReturnDouble = double (*)(v8::Local<v8::Value>,
v8::Local<v8::Value>,
const double);
using CFunctionWithInt64Fallback = void (*)(v8::Local<v8::Value>,
v8::Local<v8::Value>,
const int64_t,
v8::FastApiCallbackOptions&);
using CFunctionWithBool = void (*)(v8::Local<v8::Value>, bool);
using CFunctionWithBool = void (*)(v8::Local<v8::Value>,
v8::Local<v8::Value>,
bool);
using CFunctionWriteString =
uint32_t (*)(v8::Local<v8::Value> receiver,
@ -83,6 +93,7 @@ class ExternalReferenceRegistry {
V(CFunctionCallbackReturnDouble) \
V(CFunctionCallbackReturnInt32) \
V(CFunctionCallbackValueReturnDouble) \
V(CFunctionCallbackValueReturnDoubleUnusedReceiver) \
V(CFunctionCallbackWithInt64) \
V(CFunctionCallbackWithBool) \
V(CFunctionCallbackWithString) \

View File

@ -1040,8 +1040,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsString());
BufferValue path(env->isolate(), args[0]);
CHECK_GE(args.Length(), 2);
CHECK(args[1]->IsString());
BufferValue path(env->isolate(), args[1]);
CHECK_NOT_NULL(*path);
ToNamespacedPath(env, &path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
@ -1059,6 +1060,7 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
}
static int32_t FastInternalModuleStat(
Local<Object> unused,
Local<Object> recv,
const FastOneByteString& input,
// NOLINTNEXTLINE(runtime/references) This is V8 api.

View File

@ -72,7 +72,8 @@ class BindingData : public SnapshotableObject {
static BindingData* FromV8Value(v8::Local<v8::Value> receiver);
static void NumberImpl(BindingData* receiver);
static void FastNumber(v8::Local<v8::Value> receiver) {
static void FastNumber(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver) {
NumberImpl(FromV8Value(receiver));
}
@ -80,7 +81,8 @@ class BindingData : public SnapshotableObject {
static void BigIntImpl(BindingData* receiver);
static void FastBigInt(v8::Local<v8::Value> receiver) {
static void FastBigInt(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver) {
BigIntImpl(FromV8Value(receiver));
}

View File

@ -242,6 +242,7 @@ inline void EinvalError() {}
template <typename FT, FT F, typename R, typename... Args>
R WASI::WasiFunction<FT, F, R, Args...>::FastCallback(
Local<Object> unused,
Local<Object> receiver,
Args... args,
// NOLINTNEXTLINE(runtime/references) This is V8 api.

View File

@ -160,7 +160,8 @@ class WASI : public BaseObject,
v8::Local<v8::FunctionTemplate>);
private:
static R FastCallback(v8::Local<v8::Object> receiver,
static R FastCallback(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
Args...,
v8::FastApiCallbackOptions&);

View File

@ -33,7 +33,8 @@ void BindingData::SlowGetLibuvNow(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(Number::New(args.GetIsolate(), now));
}
double BindingData::FastGetLibuvNow(Local<Object> receiver) {
double BindingData::FastGetLibuvNow(Local<Object> unused,
Local<Object> receiver) {
return GetLibuvNowImpl(FromJSObject<BindingData>(receiver));
}
@ -47,7 +48,9 @@ void BindingData::SlowScheduleTimer(const FunctionCallbackInfo<Value>& args) {
ScheduleTimerImpl(Realm::GetBindingData<BindingData>(args), duration);
}
void BindingData::FastScheduleTimer(Local<Object> receiver, int64_t duration) {
void BindingData::FastScheduleTimer(Local<Object> unused,
Local<Object> receiver,
int64_t duration) {
ScheduleTimerImpl(FromJSObject<BindingData>(receiver), duration);
}
@ -61,7 +64,9 @@ void BindingData::SlowToggleTimerRef(
args[0]->IsTrue());
}
void BindingData::FastToggleTimerRef(Local<Object> receiver, bool ref) {
void BindingData::FastToggleTimerRef(Local<Object> unused,
Local<Object> receiver,
bool ref) {
ToggleTimerRefImpl(FromJSObject<BindingData>(receiver), ref);
}
@ -75,7 +80,9 @@ void BindingData::SlowToggleImmediateRef(
args[0]->IsTrue());
}
void BindingData::FastToggleImmediateRef(Local<Object> receiver, bool ref) {
void BindingData::FastToggleImmediateRef(Local<Object> unused,
Local<Object> receiver,
bool ref) {
ToggleImmediateRefImpl(FromJSObject<BindingData>(receiver), ref);
}

View File

@ -26,23 +26,29 @@ class BindingData : public SnapshotableObject {
static void SetupTimers(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SlowGetLibuvNow(const v8::FunctionCallbackInfo<v8::Value>& args);
static double FastGetLibuvNow(v8::Local<v8::Object> receiver);
static double FastGetLibuvNow(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver);
static double GetLibuvNowImpl(BindingData* data);
static void SlowScheduleTimer(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastScheduleTimer(v8::Local<v8::Object> receiver,
static void FastScheduleTimer(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
int64_t duration);
static void ScheduleTimerImpl(BindingData* data, int64_t duration);
static void SlowToggleTimerRef(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastToggleTimerRef(v8::Local<v8::Object> receiver, bool ref);
static void FastToggleTimerRef(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
bool ref);
static void ToggleTimerRefImpl(BindingData* data, bool ref);
static void SlowToggleImmediateRef(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastToggleImmediateRef(v8::Local<v8::Object> receiver, bool ref);
static void FastToggleImmediateRef(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
bool ref);
static void ToggleImmediateRefImpl(BindingData* data, bool ref);
static void CreatePerIsolateProperties(IsolateData* isolate_data,

View File

@ -18,7 +18,7 @@ const internalFsBinding = internalBinding('fs');
// Run this inside a for loop to trigger the fast API
for (let i = 0; i < 10_000; i++) {
assert.throws(() => {
internalFsBinding.internalModuleStat(blockedFile);
internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',

View File

@ -113,7 +113,7 @@ declare namespace InternalFSBinding {
function futimes(fd: number, atime: number, mtime: number): void;
function futimes(fd: number, atime: number, mtime: number, usePromises: typeof kUsePromises): Promise<void>;
function internalModuleStat(path: string): number;
function internalModuleStat(receiver: unknown, path: string): number;
function lchown(path: string, uid: number, gid: number, req: FSReqCallback): void;
function lchown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void;