Skip to content

Commit 0bb5584

Browse files
Ceres6targos
Ceres6
authored and
targos
committed
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: #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]>
1 parent 7186ede commit 0bb5584

18 files changed

+123
-53
lines changed

‎doc/api/cli.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ The initializer module also needs to be allowed. Consider the following example:
220220
```console
221221
$ node --experimental-permission t.js
222222
node:internal/modules/cjs/loader:162
223-
const result = internalModuleStat(filename);
223+
const result = internalModuleStat(receiver, filename);
224224
^
225225

226226
Error: Access to this API has been restricted

‎doc/api/permissions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ will be restricted.
4848
```console
4949
$ node --experimental-permission index.js
5050
node:internal/modules/cjs/loader:171
51-
const result = internalModuleStat(filename);
51+
const result = internalModuleStat(receiver, filename);
5252
^
5353

5454
Error: Access to this API has been restricted

‎doc/contributing/adding-v8-fast-api.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ for example, they may not trigger garbage collection.
2424
[`node_external_reference.h`](../../src/node_external_reference.h) file.
2525
Although, it would not start failing or crashing until the function ends up
2626
in a snapshot (either the built-in or a user-land one). Please refer to the
27-
[binding functions documentation](../../src#binding-functions) for more
27+
[binding functions documentation](../../src/README.md#binding-functions) for more
2828
information.
2929
* To test fast APIs, make sure to run the tests in a loop with a decent
3030
iterations count to trigger relevant optimizations that prefer the fast API
@@ -38,6 +38,23 @@ for example, they may not trigger garbage collection.
3838
* The fast callback must be idempotent up to the point where error and fallback
3939
conditions are checked, because otherwise executing the slow callback might
4040
produce visible side effects twice.
41+
* If the receiver is used in the callback, it must be passed as a second argument,
42+
leaving the first one unused, to prevent the JS land from accidentally omitting the receiver when
43+
invoking the fast API method.
44+
45+
```cpp
46+
// Instead of invoking the method as `receiver.internalModuleStat(input)`, the JS land should
47+
// invoke it as `internalModuleStat(binding, input)` to make sure the binding is available to
48+
// the native land.
49+
static int32_t FastInternalModuleStat(
50+
Local<Object> unused,
51+
Local<Object> recv,
52+
const FastOneByteString& input,
53+
FastApiCallbackOptions& options) {
54+
Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
55+
// More code
56+
}
57+
```
4158
4259
## Fallback to slow path
4360

‎lib/fs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1414,7 +1414,7 @@ function readdirSyncRecursive(basePath, options) {
14141414
for (let i = 0; i < readdirResult.length; i++) {
14151415
const resultPath = pathModule.join(path, readdirResult[i]);
14161416
const relativeResultPath = pathModule.relative(basePath, resultPath);
1417-
const stat = binding.internalModuleStat(resultPath);
1417+
const stat = binding.internalModuleStat(binding, resultPath);
14181418
ArrayPrototypePush(readdirResults, relativeResultPath);
14191419
// 1 indicates directory
14201420
if (stat === 1) {

‎lib/internal/fs/promises.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ async function readdirRecursive(originalPath, options) {
914914
const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
915915
for (const ent of readdir) {
916916
const direntPath = pathModule.join(path, ent);
917-
const stat = binding.internalModuleStat(direntPath);
917+
const stat = binding.internalModuleStat(binding, direntPath);
918918
ArrayPrototypePush(
919919
result,
920920
pathModule.relative(originalPath, direntPath),

‎lib/internal/modules/cjs/loader.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,9 @@ function stat(filename) {
235235
const result = statCache.get(filename);
236236
if (result !== undefined) { return result; }
237237
}
238-
const result = internalFsBinding.internalModuleStat(filename);
238+
const result = internalFsBinding.internalModuleStat(internalFsBinding, filename);
239239
if (statCache !== null && result >= 0) {
240-
// Only set cache when `internalModuleStat(filename)` succeeds.
240+
// Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
241241
statCache.set(filename, result);
242242
}
243243
return result;

‎lib/internal/modules/esm/resolve.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,10 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
241241
throw err;
242242
}
243243

244-
const stats = internalFsBinding.internalModuleStat(StringPrototypeEndsWith(path, '/') ?
245-
StringPrototypeSlice(path, -1) : path);
244+
const stats = internalFsBinding.internalModuleStat(
245+
internalFsBinding,
246+
StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path,
247+
);
246248

247249
// Check for stats.isDirectory()
248250
if (stats === 1) {
@@ -802,6 +804,7 @@ function packageResolve(specifier, base, conditions) {
802804
let lastPath;
803805
do {
804806
const stat = internalFsBinding.internalModuleStat(
807+
internalFsBinding,
805808
StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),
806809
);
807810
// Check for !stat.isDirectory()

‎src/histogram.cc

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
169169
(*histogram)->RecordDelta();
170170
}
171171

172-
void HistogramBase::FastRecordDelta(Local<Value> receiver) {
172+
void HistogramBase::FastRecordDelta(Local<Value> unused,
173+
Local<Value> receiver) {
173174
HistogramBase* histogram;
174175
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
175176
(*histogram)->RecordDelta();
@@ -189,7 +190,8 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
189190
(*histogram)->Record(value);
190191
}
191192

192-
void HistogramBase::FastRecord(Local<Value> receiver,
193+
void HistogramBase::FastRecord(Local<Value> unused,
194+
Local<Value> receiver,
193195
const int64_t value,
194196
FastApiCallbackOptions& options) {
195197
if (value < 1) {
@@ -436,7 +438,9 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
436438
histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
437439
}
438440

439-
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
441+
void IntervalHistogram::FastStart(Local<Value> unused,
442+
Local<Value> receiver,
443+
bool reset) {
440444
IntervalHistogram* histogram;
441445
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
442446
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
@@ -448,7 +452,7 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
448452
histogram->OnStop();
449453
}
450454

451-
void IntervalHistogram::FastStop(Local<Value> receiver) {
455+
void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
452456
IntervalHistogram* histogram;
453457
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
454458
histogram->OnStop();
@@ -564,42 +568,45 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
564568
(*histogram)->Reset();
565569
}
566570

567-
void HistogramImpl::FastReset(Local<Value> receiver) {
571+
void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
568572
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
569573
(*histogram)->Reset();
570574
}
571575

572-
double HistogramImpl::FastGetCount(Local<Value> receiver) {
576+
double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
573577
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
574578
return static_cast<double>((*histogram)->Count());
575579
}
576580

577-
double HistogramImpl::FastGetMin(Local<Value> receiver) {
581+
double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
578582
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
579583
return static_cast<double>((*histogram)->Min());
580584
}
581585

582-
double HistogramImpl::FastGetMax(Local<Value> receiver) {
586+
double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
583587
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
584588
return static_cast<double>((*histogram)->Max());
585589
}
586590

587-
double HistogramImpl::FastGetMean(Local<Value> receiver) {
591+
double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
588592
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
589593
return (*histogram)->Mean();
590594
}
591595

592-
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
596+
double HistogramImpl::FastGetExceeds(Local<Value> unused,
597+
Local<Value> receiver) {
593598
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
594599
return static_cast<double>((*histogram)->Exceeds());
595600
}
596601

597-
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
602+
double HistogramImpl::FastGetStddev(Local<Value> unused,
603+
Local<Value> receiver) {
598604
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
599605
return (*histogram)->Stddev();
600606
}
601607

602-
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
608+
double HistogramImpl::FastGetPercentile(Local<Value> unused,
609+
Local<Value> receiver,
603610
const double percentile) {
604611
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
605612
return static_cast<double>((*histogram)->Percentile(percentile));

‎src/histogram.h

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,22 @@ class HistogramImpl {
101101
static void GetPercentilesBigInt(
102102
const v8::FunctionCallbackInfo<v8::Value>& args);
103103

104-
static void FastReset(v8::Local<v8::Value> receiver);
105-
static double FastGetCount(v8::Local<v8::Value> receiver);
106-
static double FastGetMin(v8::Local<v8::Value> receiver);
107-
static double FastGetMax(v8::Local<v8::Value> receiver);
108-
static double FastGetMean(v8::Local<v8::Value> receiver);
109-
static double FastGetExceeds(v8::Local<v8::Value> receiver);
110-
static double FastGetStddev(v8::Local<v8::Value> receiver);
111-
static double FastGetPercentile(v8::Local<v8::Value> receiver,
104+
static void FastReset(v8::Local<v8::Value> unused,
105+
v8::Local<v8::Value> receiver);
106+
static double FastGetCount(v8::Local<v8::Value> unused,
107+
v8::Local<v8::Value> receiver);
108+
static double FastGetMin(v8::Local<v8::Value> unused,
109+
v8::Local<v8::Value> receiver);
110+
static double FastGetMax(v8::Local<v8::Value> unused,
111+
v8::Local<v8::Value> receiver);
112+
static double FastGetMean(v8::Local<v8::Value> unused,
113+
v8::Local<v8::Value> receiver);
114+
static double FastGetExceeds(v8::Local<v8::Value> unused,
115+
v8::Local<v8::Value> receiver);
116+
static double FastGetStddev(v8::Local<v8::Value> unused,
117+
v8::Local<v8::Value> receiver);
118+
static double FastGetPercentile(v8::Local<v8::Value> unused,
119+
v8::Local<v8::Value> receiver,
112120
const double percentile);
113121

114122
static void AddMethods(v8::Isolate* isolate,
@@ -158,10 +166,12 @@ class HistogramBase final : public BaseObject, public HistogramImpl {
158166
static void Add(const v8::FunctionCallbackInfo<v8::Value>& args);
159167

160168
static void FastRecord(
169+
v8::Local<v8::Value> unused,
161170
v8::Local<v8::Value> receiver,
162171
const int64_t value,
163172
v8::FastApiCallbackOptions& options); // NOLINT(runtime/references)
164-
static void FastRecordDelta(v8::Local<v8::Value> receiver);
173+
static void FastRecordDelta(v8::Local<v8::Value> unused,
174+
v8::Local<v8::Value> receiver);
165175

166176
HistogramBase(
167177
Environment* env,
@@ -233,8 +243,11 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl {
233243
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
234244
static void Stop(const v8::FunctionCallbackInfo<v8::Value>& args);
235245

236-
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
237-
static void FastStop(v8::Local<v8::Value> receiver);
246+
static void FastStart(v8::Local<v8::Value> unused,
247+
v8::Local<v8::Value> receiver,
248+
bool reset);
249+
static void FastStop(v8::Local<v8::Value> unused,
250+
v8::Local<v8::Value> receiver);
238251

239252
BaseObject::TransferMode GetTransferMode() const override {
240253
return TransferMode::kCloneable;

‎src/node_external_reference.h

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,25 @@ namespace node {
1212

1313
using CFunctionCallbackWithOneByteString =
1414
uint32_t (*)(v8::Local<v8::Value>, const v8::FastOneByteString&);
15-
using CFunctionCallback = void (*)(v8::Local<v8::Value> receiver);
15+
using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,
16+
v8::Local<v8::Value> receiver);
1617
using CFunctionCallbackReturnDouble =
17-
double (*)(v8::Local<v8::Object> receiver);
18+
double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);
1819
using CFunctionCallbackReturnInt32 =
19-
int32_t (*)(v8::Local<v8::Object> receiver,
20+
int32_t (*)(v8::Local<v8::Object> unused,
21+
v8::Local<v8::Object> receiver,
2022
const v8::FastOneByteString& input,
2123
// NOLINTNEXTLINE(runtime/references) This is V8 api.
2224
v8::FastApiCallbackOptions& options);
2325
using CFunctionCallbackValueReturnDouble =
2426
double (*)(v8::Local<v8::Value> receiver);
25-
using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> receiver,
27+
using CFunctionCallbackValueReturnDoubleUnusedReceiver =
28+
double (*)(v8::Local<v8::Value> unused, v8::Local<v8::Value> receiver);
29+
using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> unused,
30+
v8::Local<v8::Object> receiver,
2631
int64_t);
27-
using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> receiver,
32+
using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> unused,
33+
v8::Local<v8::Object> receiver,
2834
bool);
2935
using CFunctionCallbackWithString =
3036
bool (*)(v8::Local<v8::Value>, const v8::FastOneByteString& input);
@@ -50,11 +56,15 @@ using CFunctionCallbackWithUint8ArrayUint32Int64Bool =
5056
using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
5157
const uint32_t input);
5258
using CFunctionWithDoubleReturnDouble = double (*)(v8::Local<v8::Value>,
59+
v8::Local<v8::Value>,
5360
const double);
5461
using CFunctionWithInt64Fallback = void (*)(v8::Local<v8::Value>,
62+
v8::Local<v8::Value>,
5563
const int64_t,
5664
v8::FastApiCallbackOptions&);
57-
using CFunctionWithBool = void (*)(v8::Local<v8::Value>, bool);
65+
using CFunctionWithBool = void (*)(v8::Local<v8::Value>,
66+
v8::Local<v8::Value>,
67+
bool);
5868

5969
using CFunctionWriteString =
6070
uint32_t (*)(v8::Local<v8::Value> receiver,
@@ -83,6 +93,7 @@ class ExternalReferenceRegistry {
8393
V(CFunctionCallbackReturnDouble) \
8494
V(CFunctionCallbackReturnInt32) \
8595
V(CFunctionCallbackValueReturnDouble) \
96+
V(CFunctionCallbackValueReturnDoubleUnusedReceiver) \
8697
V(CFunctionCallbackWithInt64) \
8798
V(CFunctionCallbackWithBool) \
8899
V(CFunctionCallbackWithString) \

‎src/node_file.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,8 +1034,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
10341034
static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10351035
Environment* env = Environment::GetCurrent(args);
10361036

1037-
CHECK(args[0]->IsString());
1038-
BufferValue path(env->isolate(), args[0]);
1037+
CHECK_GE(args.Length(), 2);
1038+
CHECK(args[1]->IsString());
1039+
BufferValue path(env->isolate(), args[1]);
10391040
CHECK_NOT_NULL(*path);
10401041
ToNamespacedPath(env, &path);
10411042
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -1053,6 +1054,7 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10531054
}
10541055

10551056
static int32_t FastInternalModuleStat(
1057+
Local<Object> unused,
10561058
Local<Object> recv,
10571059
const FastOneByteString& input,
10581060
// NOLINTNEXTLINE(runtime/references) This is V8 api.

‎src/node_process.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,17 @@ class BindingData : public SnapshotableObject {
7272
static BindingData* FromV8Value(v8::Local<v8::Value> receiver);
7373
static void NumberImpl(BindingData* receiver);
7474

75-
static void FastNumber(v8::Local<v8::Value> receiver) {
75+
static void FastNumber(v8::Local<v8::Value> unused,
76+
v8::Local<v8::Value> receiver) {
7677
NumberImpl(FromV8Value(receiver));
7778
}
7879

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

8182
static void BigIntImpl(BindingData* receiver);
8283

83-
static void FastBigInt(v8::Local<v8::Value> receiver) {
84+
static void FastBigInt(v8::Local<v8::Value> unused,
85+
v8::Local<v8::Value> receiver) {
8486
BigIntImpl(FromV8Value(receiver));
8587
}
8688

‎src/node_wasi.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ inline void EinvalError() {}
241241

242242
template <typename FT, FT F, typename R, typename... Args>
243243
R WASI::WasiFunction<FT, F, R, Args...>::FastCallback(
244+
Local<Object> unused,
244245
Local<Object> receiver,
245246
Args... args,
246247
// NOLINTNEXTLINE(runtime/references) This is V8 api.

‎src/node_wasi.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ class WASI : public BaseObject,
160160
v8::Local<v8::FunctionTemplate>);
161161

162162
private:
163-
static R FastCallback(v8::Local<v8::Object> receiver,
163+
static R FastCallback(v8::Local<v8::Object> unused,
164+
v8::Local<v8::Object> receiver,
164165
Args...,
165166
v8::FastApiCallbackOptions&);
166167

0 commit comments

Comments
 (0)