Skip to content

Commit

Permalink
src: add receiver to fast api callback methods
Browse files Browse the repository at this point in the history
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: #54408
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
  • Loading branch information
Ceres6 authored and targos committed Oct 4, 2024
1 parent 4bd3c2f commit 6a18b93
Showing 18 changed files with 123 additions and 53 deletions.
2 changes: 1 addition & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion doc/api/permissions.md
Original file line number Diff line number Diff line change
@@ -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
19 changes: 18 additions & 1 deletion doc/contributing/adding-v8-fast-api.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
@@ -1414,7 +1414,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) {
2 changes: 1 addition & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
@@ -914,7 +914,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),
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 5 additions & 2 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
@@ -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()
31 changes: 19 additions & 12 deletions src/histogram.cc
Original file line number Diff line number Diff line change
@@ -169,7 +169,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();
@@ -189,7 +190,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) {
@@ -436,7 +438,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);
@@ -448,7 +452,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();
@@ -564,42 +568,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));
35 changes: 24 additions & 11 deletions src/histogram.h
Original file line number Diff line number Diff line change
@@ -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;
23 changes: 17 additions & 6 deletions src/node_external_reference.h
Original file line number Diff line number Diff line change
@@ -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) \
6 changes: 4 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
@@ -1034,8 +1034,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(
@@ -1053,6 +1054,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.
6 changes: 4 additions & 2 deletions src/node_process.h
Original file line number Diff line number Diff line change
@@ -72,15 +72,17 @@ 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));
}

static void SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args);

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));
}

1 change: 1 addition & 0 deletions src/node_wasi.cc
Original file line number Diff line number Diff line change
@@ -241,6 +241,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.
3 changes: 2 additions & 1 deletion src/node_wasi.h
Original file line number Diff line number Diff line change
@@ -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&);

Loading

0 comments on commit 6a18b93

Please sign in to comment.