Skip to content

Commit f5d454a

Browse files
Ceres6Ceres6
authored
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 18acff0 commit f5d454a

File tree

18 files changed

+123
-53
lines changed

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
@@ -1411,7 +1411,7 @@ function readdirSyncRecursive(basePath, options) {
14111411
for (let i = 0; i < readdirResult.length; i++) {
14121412
const resultPath = pathModule.join(path, readdirResult[i]);
14131413
const relativeResultPath = pathModule.relative(basePath, resultPath);
1414-
const stat = binding.internalModuleStat(resultPath);
1414+
const stat = binding.internalModuleStat(binding, resultPath);
14151415
ArrayPrototypePush(readdirResults, relativeResultPath);
14161416
// 1 indicates directory
14171417
if (stat === 1) {

‎lib/internal/fs/promises.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ async function readdirRecursive(originalPath, options) {
915915
const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
916916
for (const ent of readdir) {
917917
const direntPath = pathModule.join(path, ent);
918-
const stat = binding.internalModuleStat(direntPath);
918+
const stat = binding.internalModuleStat(binding, direntPath);
919919
ArrayPrototypePush(
920920
result,
921921
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
@@ -170,7 +170,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
170170
(*histogram)->RecordDelta();
171171
}
172172

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

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

441-
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
443+
void IntervalHistogram::FastStart(Local<Value> unused,
444+
Local<Value> receiver,
445+
bool reset) {
442446
IntervalHistogram* histogram;
443447
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
444448
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
@@ -450,7 +454,7 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
450454
histogram->OnStop();
451455
}
452456

453-
void IntervalHistogram::FastStop(Local<Value> receiver) {
457+
void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
454458
IntervalHistogram* histogram;
455459
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
456460
histogram->OnStop();
@@ -566,42 +570,45 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
566570
(*histogram)->Reset();
567571
}
568572

569-
void HistogramImpl::FastReset(Local<Value> receiver) {
573+
void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
570574
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
571575
(*histogram)->Reset();
572576
}
573577

574-
double HistogramImpl::FastGetCount(Local<Value> receiver) {
578+
double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
575579
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
576580
return static_cast<double>((*histogram)->Count());
577581
}
578582

579-
double HistogramImpl::FastGetMin(Local<Value> receiver) {
583+
double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
580584
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
581585
return static_cast<double>((*histogram)->Min());
582586
}
583587

584-
double HistogramImpl::FastGetMax(Local<Value> receiver) {
588+
double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
585589
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
586590
return static_cast<double>((*histogram)->Max());
587591
}
588592

589-
double HistogramImpl::FastGetMean(Local<Value> receiver) {
593+
double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
590594
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
591595
return (*histogram)->Mean();
592596
}
593597

594-
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
598+
double HistogramImpl::FastGetExceeds(Local<Value> unused,
599+
Local<Value> receiver) {
595600
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
596601
return static_cast<double>((*histogram)->Exceeds());
597602
}
598603

599-
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
604+
double HistogramImpl::FastGetStddev(Local<Value> unused,
605+
Local<Value> receiver) {
600606
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
601607
return (*histogram)->Stddev();
602608
}
603609

604-
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
610+
double HistogramImpl::FastGetPercentile(Local<Value> unused,
611+
Local<Value> receiver,
605612
const double percentile) {
606613
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
607614
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) \

0 commit comments

Comments
 (0)