Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: add receiver to fast api callback methods #54408

Merged
merged 4 commits into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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
Expand Down
15 changes: 14 additions & 1 deletion doc/contributing/adding-v8-fast-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -38,6 +38,19 @@ 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.
Ceres6 marked this conversation as resolved.
Show resolved Hide resolved

```cpp
static int32_t FastInternalModuleStat(
Ceres6 marked this conversation as resolved.
Show resolved Hide resolved
Local<Object> unused,
Local<Object> recv,
const FastOneByteString& input,
FastApiCallbackOptions& options) {
Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
// More code
}
```

## Fallback to slow path

Expand Down
2 changes: 1 addition & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1410,7 +1410,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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,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;
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,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) {
Expand Down Expand Up @@ -804,6 +806,7 @@ function packageResolve(specifier, base, conditions) {
let lastPath;
do {
const stat = internalFsBinding.internalModuleStat(
internalFsBinding,
StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),
);
// Check for !stat.isDirectory()
Expand Down
3 changes: 2 additions & 1 deletion src/node_external_reference.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ using CFunctionCallback = void (*)(v8::Local<v8::Value> receiver);
using CFunctionCallbackReturnDouble =
double (*)(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);
Expand Down
6 changes: 4 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-permission-fs-internal-module-stat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion typings/internalBinding/fs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading