diff --git a/lib/fs.js b/lib/fs.js index 81c4d2b9c884..cfed38d8127e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -81,6 +81,7 @@ const { const { FSReqCallback, + ReadFileJob, } = binding; const { toPathIfFileURL } = require('internal/url'); const { @@ -98,6 +99,7 @@ const { const { constants: { kIoMaxLength, + kReadFileBufferLength, kMaxUserId, }, copyObject, @@ -428,10 +430,70 @@ function readFile(path, options, callback) { return; const flagsNumber = stringToFlags(options.flag, 'options.flag'); + path = getValidatedPath(path); + if (options.buffer === undefined) { + // Open + fstat + read + close in one thread pool round trip for files of + // up to one chunk; larger files come back as an open fd + size and take + // the chunked reader below (readFileAfterOneShot). `true`: a handed-back + // fd will be closed through fs.close(), so track it as unmanaged. + const job = new ReadFileJob(path, flagsNumber, kReadFileBufferLength, true); + job.context = context; + job.ondone = readFileAfterOneShot; + const accessError = job.run(path); + if (accessError !== undefined) { + // Not scheduled: report it the way the request-based open() did. + callback(accessError); + } + return; + } const req = new FSReqCallback(); req.context = context; req.oncomplete = readFileAfterOpen; - binding.open(getValidatedPath(path), flagsNumber, 0o666, req); + binding.open(path, flagsNumber, 0o666, req); +} + +function readFileAfterOneShot(err, buffer, fd, size, closeErr) { + const context = this.context; + if (err) { + context.callback(err); + return; + } + if (fd !== -1) { + // (context.read() below performs the abort check for this case.) + // Larger than one chunk: continue exactly like after open + fstat. + context.fd = fd; + context.size = size; + if (size > kIoMaxLength) { + return context.close(new ERR_FS_FILE_TOO_LARGE(size)); + } + try { + context.prepare(); + } catch (err) { + return context.close(err); + } + context.read(); + return; + } + if (closeErr) { + context.callback(closeErr); + return; + } + if (context.signal?.aborted) { + // An abort that arrived while the read was in flight wins, as it did when + // it was noticed between the open/fstat/read steps. + context.callback(new AbortError(undefined, { cause: context.signal.reason })); + return; + } + let result = buffer; + if (context.encoding) { + try { + result = buffer.toString(context.encoding); + } catch (err) { + context.callback(err); + return; + } + } + context.callback(null, result); } function tryStatSync(fd, isUserFd) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 3e336024a15a..d571de820ced 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1211,26 +1211,32 @@ async function readFileHandleWithUserBuffer(filehandle, options, size) { return encoding ? buffer.toString(encoding) : buffer.subarray(0, totalRead); } -async function readFileHandle(filehandle, options) { +async function readFileHandle(filehandle, options, knownRegularFileSize) { const signal = options?.signal; const encoding = options?.encoding; const decoder = encoding && new StringDecoder(encoding); checkAborted(signal); - const statFields = await PromisePrototypeThen( - binding.fstat(filehandle.fd, false, kUsePromises), - undefined, - handleErrorFromBinding, - ); - - checkAborted(signal); - let size = 0; let length = 0; - if ((statFields[1/* mode */] & S_IFMT) === S_IFREG) { - size = statFields[8/* size */]; + if (knownRegularFileSize !== undefined) { + // Handed over by readFile() together with an already open fd. + size = knownRegularFileSize; length = encoding ? MathMin(size, kReadFileBufferLength) : size; + } else { + const statFields = await PromisePrototypeThen( + binding.fstat(filehandle.fd, false, kUsePromises), + undefined, + handleErrorFromBinding, + ); + + checkAborted(signal); + + if ((statFields[1/* mode */] & S_IFMT) === S_IFREG) { + size = statFields[8/* size */]; + length = encoding ? MathMin(size, kReadFileBufferLength) : size; + } } if (length === 0) { length = kReadFileUnknownBufferLength; @@ -2146,10 +2152,49 @@ async function readFile(path, options) { checkAborted(options.signal); + if (options.buffer === undefined && vfsState.handlers === null) { + // Open + fstat + read + close in one thread pool round trip for files of + // up to one chunk; larger files come back as an open fd + size and are + // read by readFileHandle() as before. + path = getValidatedPath(path); + const { 0: buffer, 1: fd, 2: size } = await readFileInOneRoundTrip(path, stringToFlags(flag)); + if (fd === -1) { + checkAborted(options.signal); // An abort during the read still wins. + return options.encoding ? buffer.toString(options.encoding) : buffer; + } + const filehandle = new FileHandle(new binding.FileHandle(fd)); + return handleFdClose(readFileHandle(filehandle, options, size), filehandle.close); + } + const fd = await open(path, flag, 0o666); return handleFdClose(readFileHandle(fd, options), fd.close); } +/** + * @param {string|Buffer} path Validated path + * @param {number} flagsNumber + * @returns {Promise<[Buffer|undefined, number, number|undefined]>} [buffer, -1] or [undefined, fd, size] + */ +function readFileInOneRoundTrip(path, flagsNumber) { + return new Promise((resolve, reject) => { + const job = new binding.ReadFileJob(path, flagsNumber, kReadFileBufferLength); + job.ondone = (err, buffer, fd, size, closeErr) => { + const error = err ?? closeErr; + if (error != null) { + ErrorCaptureStackTrace(error, readFileInOneRoundTrip); + reject(error); + } else { + resolve([buffer, fd, size]); + } + }; + const accessError = job.run(path); + if (accessError !== undefined) { + ErrorCaptureStackTrace(accessError, readFileInOneRoundTrip); + reject(accessError); + } + }); +} + async function* _watch(filename, options = kEmptyObject) { const h = vfsState.handlers; if (h !== null) { diff --git a/src/node_file.cc b/src/node_file.cc index ae0d9f34f8e1..a45654aab7e2 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -40,6 +40,7 @@ #include "req_wrap-inl.h" #include "stream_base-inl.h" #include "string_bytes.h" +#include "threadpoolwork-inl.h" #include "uv.h" #include "v8-fast-api-calls.h" @@ -78,6 +79,7 @@ using v8::LocalVector; using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; +using v8::Null; using v8::Number; using v8::Object; using v8::ObjectTemplate; @@ -2952,6 +2954,283 @@ static void ReadFileUtf8(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(val); } +// Reads a whole (small) file in ONE thread pool round trip -- open + fstat + +// read + close -- instead of one round trip per step, which is what dominates +// fs.readFile() for the typical small file and multiplies thread pool +// contention when many files are read at once. Files whose size exceeds +// `limit` are not read here: the job hands the open fd and the size back so +// that the caller continues with the chunked reader (which stays fair and +// abortable for large files); it then only saved the fstat() round trip. +// +// JS: const job = new ReadFileJob(path, flags, limit, trackFd); +// job.ondone = (err, buffer, fd, size, closeErr) => {...}; job.run(path); +// Exactly one of these outcomes is reported: +// err -- open/fstat/read failed (any fd opened here was closed); +// fd >= 0, size -- file is larger than `limit`: caller owns fd now (it is +// registered as an unmanaged fd iff trackFd, i.e. when the +// caller will close it through fs.close() rather than a +// FileHandle); +// buffer -- the whole content; closeErr set if only close() failed. +class ReadFileJob final : public AsyncWrap, public ThreadPoolWork { + public: + static void New(const FunctionCallbackInfo& args) { + CHECK(args.IsConstructCall()); + Environment* env = Environment::GetCurrent(args); + CHECK_GE(args.Length(), 3); + BufferValue path(env->isolate(), args[0]); + CHECK_NOT_NULL(*path); + ToNamespacedPath(env, &path); + CHECK(args[1]->IsInt32()); + CHECK(args[2]->IsNumber()); + const int flags = args[1].As()->Value(); + const double limit = args[2].As()->Value(); + const bool track_fd = args.Length() > 3 && args[3]->IsTrue(); + new ReadFileJob(env, + args.This(), + path.ToString(), + flags, + limit < 0 ? 0 : static_cast(limit), + track_fd); + } + + // Returns undefined when the job was scheduled, or the ERR_ACCESS_DENIED + // error the asynchronous open() would have delivered through its request + // (nothing is scheduled then; the caller passes it to the callback). + static void Run(const FunctionCallbackInfo& args) { + ReadFileJob* job; + ASSIGN_OR_RETURN_UNWRAP(&job, args.This()); + Environment* env = job->AsyncWrap::env(); + CHECK(!job->scheduled_); + BufferValue path(env->isolate(), args[0]); + CHECK_NOT_NULL(*path); + ToNamespacedPath(env, &path); + Local access_error; + if (OpenPermissionError(env, path, job->flags_).ToLocal(&access_error)) { + args.GetReturnValue().Set(access_error); + return; + } + job->scheduled_ = true; + // Keep the wrapper alive while the work is in flight. + job->ClearWeak(); + FS_ASYNC_TRACE_BEGIN0(UV_FS_READ, job) + job->ScheduleWork(); + } + + void DoThreadPoolWork() override { + uv_fs_t req; + int fd = uv_fs_open(nullptr, &req, path_.c_str(), flags_, 0666, nullptr); + uv_fs_req_cleanup(&req); + if (fd < 0) return Fail("open", fd); + + int rc = uv_fs_fstat(nullptr, &req, fd, nullptr); + if (rc < 0) { + uv_fs_req_cleanup(&req); + Fail("fstat", rc); + CloseQuietly(fd); + return; + } + const uv_stat_t* const st = static_cast(req.ptr); + const bool regular = (st->st_mode & S_IFMT) == S_IFREG; + const uint64_t size = regular ? static_cast(st->st_size) : 0; + uv_fs_req_cleanup(&req); + + if (size > limit_) { + // Too large to read in one go here: hand the fd back. + fd_ = fd; + size_ = size; + return; + } + + // Known size: read exactly that much (like the chunked reader, stop when + // it has been read or at EOF, whichever comes first). Unknown size (0, + // e.g. procfs): grow until EOF. + size_t cap = size > 0 ? static_cast(size) : kUnknownSizeChunk; + data_ = UncheckedMalloc(cap); + if (data_ == nullptr) { + Fail("read", UV_ENOMEM); + CloseQuietly(fd); + return; + } + while (true) { + if (len_ == cap) { + if (size > 0) break; // Read all of the announced size. + // A file that claims size 0 keeps growing until EOF (or ENOMEM), + // like the chunked reader's buffer list did. + size_t new_cap = cap * 2; + char* grown = UncheckedRealloc(data_, new_cap); + if (grown == nullptr) { + Fail("read", UV_ENOMEM); + break; + } + data_ = grown; + cap = new_cap; + } + uv_buf_t buf = uv_buf_init(data_ + len_, + static_cast(std::min( + cap - len_, kMaxReadChunk))); + int r = uv_fs_read(nullptr, &req, fd, &buf, 1, -1, nullptr); + uv_fs_req_cleanup(&req); + if (r < 0) { + Fail("read", r); + break; + } + if (r == 0) break; + len_ += static_cast(r); + } + + rc = uv_fs_close(nullptr, &req, fd, nullptr); + uv_fs_req_cleanup(&req); + if (rc < 0) close_error_ = rc; + if (error_ != 0) { + free(data_); + data_ = nullptr; + len_ = 0; + } else if (cap - len_ >= 4096) { + // Do not retain the slack of a size-0 (grown) or short file. + char* shrunk = UncheckedRealloc(data_, len_ > 0 ? len_ : 1); + if (shrunk != nullptr) data_ = shrunk; + } + } + + void AfterThreadPoolWork(int status) override { + Environment* env = AsyncWrap::env(); + std::unique_ptr self(this); + CHECK(status == 0 || status == UV_ECANCELED); + FS_ASYNC_TRACE_END0(UV_FS_READ, this) + if (status == UV_ECANCELED) { + if (fd_ >= 0) CloseQuietly(fd_); + return; + } + if (!env->can_call_into_js()) { + if (fd_ >= 0) CloseQuietly(fd_); + return; + } + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + Isolate* isolate = env->isolate(); + + Local argv[5] = {Null(isolate), + Undefined(isolate), + Integer::New(isolate, -1), + Undefined(isolate), + Undefined(isolate)}; + if (error_ != 0) { + argv[0] = UVException(isolate, error_, syscall_, nullptr, path_.c_str()); + } else if (fd_ >= 0) { + if (track_fd_) env->AddUnmanagedFd(fd_); + argv[2] = Integer::New(isolate, fd_); + argv[3] = Number::New(isolate, static_cast(size_)); + fd_ = -1; + } else { + Local buffer; + char* data = data_; + data_ = nullptr; + if (!Buffer::New(env, data, len_).ToLocal(&buffer)) { + // Buffer::New took ownership of data either way. + argv[0] = ERR_MEMORY_ALLOCATION_FAILED(isolate); + } else { + argv[1] = buffer; + } + if (close_error_ != 0) { + argv[4] = UVException(isolate, close_error_, "close"); + } + } + MakeCallback(env->ondone_string(), arraysize(argv), argv); + } + + ~ReadFileJob() override { + free(data_); + if (fd_ >= 0) CloseQuietly(fd_); + } + + bool IsNotIndicativeOfMemoryLeakAtExit() const override { return true; } + SET_NO_MEMORY_INFO() + SET_MEMORY_INFO_NAME(ReadFileJob) + SET_SELF_SIZE(ReadFileJob) + + private: + static constexpr size_t kUnknownSizeChunk = 64 * 1024; + static constexpr size_t kMaxReadChunk = 256 * 1024 * 1024; + + ReadFileJob(Environment* env, + Local object, + std::string&& path, + int flags, + uint64_t limit, + bool track_fd) + : AsyncWrap(env, object, AsyncWrap::PROVIDER_FSREQCALLBACK), + ThreadPoolWork(env, "fs.readfile"), + path_(std::move(path)), + flags_(flags), + limit_(limit), + track_fd_(track_fd) { + MakeWeak(); + } + + void Fail(const char* syscall, int error) { + syscall_ = syscall; + error_ = error; + } + + // The permission checks AsyncCheckOpenPermissions() performs for open(), + // producing the error object instead of rejecting a request wrap. + static MaybeLocal OpenPermissionError(Environment* env, + const BufferValue& path, + int flags) { + if (!env->permission()->enabled()) [[likely]] + return {}; + const int rwflags = + flags & (UV_FS_O_RDONLY | UV_FS_O_WRONLY | UV_FS_O_RDWR); + const int write_as_side_effect = + flags & + (UV_FS_O_APPEND | UV_FS_O_CREAT | UV_FS_O_TRUNC | UV_FS_O_TEMPORARY); + const auto path_view = path.ToStringView(); + auto denied = [&](permission::PermissionScope scope) -> MaybeLocal { + if (env->permission()->is_granted(env, scope, path_view) || + env->permission()->warning_only()) { + return {}; + } + Local err; + if (permission::CreateAccessDeniedError(env, scope, path_view) + .ToLocal(&err)) { + return err; + } + return Integer::New(env->isolate(), UV_EACCES); + }; + if (rwflags != UV_FS_O_WRONLY) { + MaybeLocal err = + denied(permission::PermissionScope::kFileSystemRead); + if (!err.IsEmpty()) return err; + } + if (rwflags != UV_FS_O_RDONLY || write_as_side_effect) { + MaybeLocal err = + denied(permission::PermissionScope::kFileSystemWrite); + if (!err.IsEmpty()) return err; + } + return {}; + } + + static void CloseQuietly(int fd) { + uv_fs_t req; + uv_fs_close(nullptr, &req, fd, nullptr); + uv_fs_req_cleanup(&req); + } + + std::string path_; + int flags_; + uint64_t limit_; + bool track_fd_; + bool scheduled_ = false; + // Results (written on the thread pool thread, read on the loop thread). + const char* syscall_ = nullptr; + int error_ = 0; + int close_error_ = 0; + char* data_ = nullptr; + size_t len_ = 0; + int fd_ = -1; + uint64_t size_ = 0; +}; + // Wrapper for readv(2). // // bytesRead = fs.readv(fd, buffers[, position], callback) @@ -4259,6 +4538,14 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, Integer::New(isolate, static_cast(FsStatsOffset::kFsStatsFieldsNumber))); + // Create FunctionTemplate for ReadFileJob + Local rfj = NewFunctionTemplate(isolate, ReadFileJob::New); + rfj->InstanceTemplate()->SetInternalFieldCount( + ReadFileJob::kInternalFieldCount); + rfj->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data)); + SetProtoMethod(isolate, rfj, "run", ReadFileJob::Run); + SetConstructorFunction(isolate, target, "ReadFileJob", rfj); + // Create FunctionTemplate for FSReqCallback Local fst = NewFunctionTemplate(isolate, NewFSReqCallback); fst->InstanceTemplate()->SetInternalFieldCount( @@ -4330,6 +4617,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Close); registry->Register(ExistsSync); registry->Register(Open); + registry->Register(ReadFileJob::New); + registry->Register(ReadFileJob::Run); registry->Register(OpenFileHandle); registry->Register(Read); registry->Register(ReadFileUtf8); diff --git a/test/async-hooks/test-async-exec-resource-match.js b/test/async-hooks/test-async-exec-resource-match.js index 6174768ba768..0b68b30ffa93 100644 --- a/test/async-hooks/test-async-exec-resource-match.js +++ b/test/async-hooks/test-async-exec-resource-match.js @@ -12,7 +12,9 @@ const { // Ignore any asyncIds created before our hook is active. let firstSeenAsyncId = -1; const idResMap = new Map(); -const numExpectedCalls = 5; +// The AsyncResource below plus at least one FSREQCALLBACK from readFile() +// (a small file is read in a single request). +const numExpectedCalls = 2; createHook({ init: common.mustCallAtLeast( diff --git a/test/async-hooks/test-fsreqcallback-readFile.js b/test/async-hooks/test-fsreqcallback-readFile.js index 65f3652f12f9..fb84306cfa86 100644 --- a/test/async-hooks/test-fsreqcallback-readFile.js +++ b/test/async-hooks/test-fsreqcallback-readFile.js @@ -18,7 +18,12 @@ hooks.enable(); fs.readFile(__filename, common.mustCall(onread)); function onread() { + // fs.readFile() of a small file is a single request (open + fstat + read + + // close in one thread pool round trip); larger files continue with one + // request per chunk. Either way each request is an FSREQCALLBACK triggered + // by the previous one (or by the top-level for the first). const as = hooks.activitiesOfTypes('FSREQCALLBACK'); + assert.ok(as.length >= 1); let lastParent = 1; for (let i = 0; i < as.length; i++) { const a = as[i]; @@ -27,17 +32,15 @@ function onread() { assert.strictEqual(a.triggerAsyncId, lastParent); lastParent = a.uid; } - checkInvocations(as[0], { init: 1, before: 1, after: 1, destroy: 1 }, - 'reqwrap[0]: while in onread callback'); - checkInvocations(as[1], { init: 1, before: 1, after: 1, destroy: 1 }, - 'reqwrap[1]: while in onread callback'); - checkInvocations(as[2], { init: 1, before: 1, after: 1, destroy: 1 }, - 'reqwrap[2]: while in onread callback'); + for (let i = 0; i < as.length - 1; i++) { + checkInvocations(as[i], { init: 1, before: 1, after: 1, destroy: 1 }, + `reqwrap[${i}]: while in onread callback`); + } // This callback is called from within the last fs req callback therefore // the last req is still going and after/destroy haven't been called yet - checkInvocations(as[3], { init: 1, before: 1 }, - 'reqwrap[3]: while in onread callback'); + checkInvocations(as[as.length - 1], { init: 1, before: 1 }, + `reqwrap[${as.length - 1}]: while in onread callback`); tick(2); } diff --git a/test/async-hooks/test-graph.fsreq-readFile.js b/test/async-hooks/test-graph.fsreq-readFile.js index 543ce68aa736..39435cdc4891 100644 --- a/test/async-hooks/test-graph.fsreq-readFile.js +++ b/test/async-hooks/test-graph.fsreq-readFile.js @@ -5,10 +5,19 @@ const initHooks = require('./init-hooks'); const verifyGraph = require('./verify-graph'); const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); + +// A file just over one read chunk (512 KiB): the first request opens, stats +// and hands the fd back, then two chunked reads and a close follow, each +// triggered by the previous request. (Smaller files are a single request.) +tmpdir.refresh(); +const file = tmpdir.resolve('graph-readfile.bin'); +fs.writeFileSync(file, Buffer.alloc(512 * 1024 + 1, 'x')); + const hooks = initHooks(); hooks.enable(); -fs.readFile(__filename, common.mustCall(onread)); +fs.readFile(file, common.mustCall(onread)); function onread() {} diff --git a/test/parallel/test-fs-promises-file-handle-aggregate-errors.js b/test/parallel/test-fs-promises-file-handle-aggregate-errors.js index f53ce1eeaf0d..36ebc23491ca 100644 --- a/test/parallel/test-fs-promises-file-handle-aggregate-errors.js +++ b/test/parallel/test-fs-promises-file-handle-aggregate-errors.js @@ -23,7 +23,10 @@ const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); let count = 0; async function createFile() { const filePath = tmpdir.resolve(`aggregate_errors_${++count}.txt`); - await writeFile(filePath, 'content'); + // Larger than one read chunk (512 KiB), so that readFile(path) reads it + // through a FileHandle (small files are read in a single native round trip + // that does not involve FileHandle.prototype). + await writeFile(filePath, 'content'.repeat(100_000)); return filePath; } diff --git a/test/parallel/test-fs-promises-file-handle-close-errors.js b/test/parallel/test-fs-promises-file-handle-close-errors.js index 8d0a1bad4605..901e71c15ac5 100644 --- a/test/parallel/test-fs-promises-file-handle-close-errors.js +++ b/test/parallel/test-fs-promises-file-handle-close-errors.js @@ -23,7 +23,10 @@ const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); let count = 0; async function createFile() { const filePath = tmpdir.resolve(`close_errors_${++count}.txt`); - await writeFile(filePath, 'content'); + // Larger than one read chunk (512 KiB), so that readFile(path) reads it + // through a FileHandle (small files are read in a single native round trip + // that does not involve FileHandle.prototype). + await writeFile(filePath, 'content'.repeat(100_000)); return filePath; } diff --git a/test/parallel/test-fs-promises-file-handle-op-errors.js b/test/parallel/test-fs-promises-file-handle-op-errors.js index d87769cd7c47..46b4acd0b8ff 100644 --- a/test/parallel/test-fs-promises-file-handle-op-errors.js +++ b/test/parallel/test-fs-promises-file-handle-op-errors.js @@ -23,7 +23,10 @@ const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); let count = 0; async function createFile() { const filePath = tmpdir.resolve(`op_errors_${++count}.txt`); - await writeFile(filePath, 'content'); + // Larger than one read chunk (512 KiB), so that readFile(path) reads it + // through a FileHandle (small files are read in a single native round trip + // that does not involve FileHandle.prototype). + await writeFile(filePath, 'content'.repeat(100_000)); return filePath; } diff --git a/test/parallel/test-fs-promises-readfile.js b/test/parallel/test-fs-promises-readfile.js index ccf7aa16b12e..0efde29339c6 100644 --- a/test/parallel/test-fs-promises-readfile.js +++ b/test/parallel/test-fs-promises-readfile.js @@ -4,7 +4,7 @@ const common = require('../common'); const assert = require('assert'); -const { writeFile, readFile } = require('fs').promises; +const { open, writeFile, readFile } = require('fs').promises; const tmpdir = require('../common/tmpdir'); const { internalBinding } = require('internal/test/binding'); const fsBinding = internalBinding('fs'); @@ -70,13 +70,21 @@ async function validateWrongSignalParam() { } async function validateZeroByteLiar() { + // readFile(path) sizes and reads small files natively (a lying size is + // handled there, see validateReadFileProc); the JS chunked reader that + // FileHandles use must cope with a file that claims size 0 as well. const originalFStat = fsBinding.fstat; fsBinding.fstat = common.mustCall( async () => (/* stat fields */ [0, 1, 2, 3, 4, 5, 6, 7, 0 /* size */]) ); - const readBuffer = await readFile(fn); - assert.strictEqual(readBuffer.toString(), largeBuffer.toString()); - fsBinding.fstat = originalFStat; + const fh = await open(fn); + try { + const readBuffer = await readFile(fh); + assert.strictEqual(readBuffer.toString(), largeBuffer.toString()); + } finally { + fsBinding.fstat = originalFStat; + await fh.close(); + } } (async () => { diff --git a/test/parallel/test-fs-readfile-one-roundtrip.js b/test/parallel/test-fs-readfile-one-roundtrip.js new file mode 100644 index 000000000000..7f73322a97d8 --- /dev/null +++ b/test/parallel/test-fs-readfile-one-roundtrip.js @@ -0,0 +1,152 @@ +'use strict'; +// fs.readFile()/fs.promises.readFile() read files of up to one chunk +// (512 KiB) with a single thread pool round trip and hand larger files over +// to the chunked reader. This pins the observable behaviour around that +// boundary: contents, encodings, empty and size-misreporting files, error +// shapes (which syscall failed), abort handling, flags, and that file +// descriptors and user buffers keep taking their existing paths. +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const fs = require('fs'); +const { promisify } = require('util'); +// Callback API through a promise, so both APIs can be awaited the same way. +const readFileCb = promisify(fs.readFile); +const async_hooks = require('async_hooks'); + +tmpdir.refresh(); + +const kChunk = 512 * 1024; +const sizes = [0, 1, 4096, kChunk - 1, kChunk, kChunk + 1, 3 * kChunk + 17]; +const files = new Map(); +for (const size of sizes) { + const file = tmpdir.resolve(`f-${size}.bin`); + const buf = Buffer.alloc(size); + for (let i = 0; i < size; i++) buf[i] = (i * 31 + size) & 0xff; + fs.writeFileSync(file, buf); + files.set(size, { file, buf }); +} +const textFile = tmpdir.resolve('text.txt'); +const text = 'héllo wörld ✓ \u{1F600}\n'.repeat(1000); +fs.writeFileSync(textFile, text); + +async function main() { + // Contents across the one-shot / chunked boundary, both APIs. + for (const [size, { file, buf }] of files) { + assert.deepStrictEqual(await fs.promises.readFile(file), buf, `promises size=${size}`); + assert.deepStrictEqual(await new Promise((res, rej) => fs.readFile(file, (e, d) => (e ? rej(e) : res(d)))), buf, + `callback size=${size}`); + // Explicit flags (string and numeric). + assert.deepStrictEqual(await fs.promises.readFile(file, { flag: 'r' }), buf); + assert.deepStrictEqual(await readFileCb(file, { flag: fs.constants.O_RDONLY }), buf); + } + + // Encodings. + assert.strictEqual(await fs.promises.readFile(textFile, 'utf8'), text); + assert.strictEqual(await fs.promises.readFile(textFile, { encoding: 'latin1' }), + Buffer.from(text).toString('latin1')); + assert.strictEqual(await readFileCb(textFile, 'base64'), Buffer.from(text).toString('base64')); + fs.readFile(textFile, 'utf8', common.mustSucceed((d) => assert.strictEqual(d, text))); + + // Errors keep their syscall/code/path shape. + const missing = tmpdir.resolve('does-not-exist'); + for (const read of [() => fs.promises.readFile(missing), + () => new Promise((res, rej) => fs.readFile(missing, (e) => (e ? rej(e) : res())))]) { + await assert.rejects(read, (err) => { + assert.strictEqual(err.code, 'ENOENT'); + assert.strictEqual(err.syscall, 'open'); + assert.strictEqual(err.path, missing); + assert.match(err.message, /ENOENT: no such file or directory, open/); + return true; + }); + } + if (!common.isWindows) { + // Reading a directory: open succeeds, read fails (as before: syscall 'read'). + for (const read of [() => fs.promises.readFile(tmpdir.path), + () => new Promise((res, rej) => fs.readFile(tmpdir.path, (e) => (e ? rej(e) : res())))]) { + await assert.rejects(read, { code: 'EISDIR', syscall: 'read' }); + } + } + // Abort: already-aborted signals reject before touching the file; both APIs. + { + const signal = AbortSignal.abort(); + await assert.rejects(fs.promises.readFile(textFile, { signal }), { name: 'AbortError' }); + await assert.rejects(readFileCb(textFile, { signal }), { name: 'AbortError' }); + // Aborting during a large (chunked) read still works. + const ac = new AbortController(); + const big = files.get(3 * kChunk + 17).file; + const p = fs.promises.readFile(big, { signal: ac.signal }); + ac.abort(); + await assert.rejects(p, { name: 'AbortError' }); + } + + // File descriptors and FileHandles keep working (they take the existing path). + { + const { file, buf } = files.get(4096); + const fd = fs.openSync(file, 'r'); + assert.deepStrictEqual(await new Promise((res, rej) => fs.readFile(fd, (e, d) => (e ? rej(e) : res(d)))), buf); + fs.closeSync(fd); + const fh = await fs.promises.open(file, 'r'); + assert.deepStrictEqual(await fh.readFile(), buf); + assert.deepStrictEqual(await fs.promises.readFile(fh), Buffer.alloc(0)); // Position is at EOF now. + await fh.close(); + } + + // Large files handed back to the chunked reader must not leak the fd: + // read one many times and make sure we can still open files afterwards. + { + const { file, buf } = files.get(kChunk + 1); + for (let i = 0; i < 64; i++) { + assert.strictEqual((await fs.promises.readFile(file)).length, buf.length); + } + await Promise.all(Array.from({ length: 64 }, () => readFileCb(file))); + } + + // Files whose reported size is wrong (Linux procfs/sysfs) are read completely. + if (common.isLinux) { + for (const file of ['/proc/self/status', '/proc/self/maps', '/sys/kernel/mm/transparent_hugepage/enabled']) { + let sync; + try { sync = fs.readFileSync(file); } catch { continue; } + const viaPromise = await fs.promises.readFile(file); + const viaCallback = await new Promise((res, rej) => fs.readFile(file, (e, d) => (e ? rej(e) : res(d)))); + if (file === '/sys/kernel/mm/transparent_hugepage/enabled') { + assert.deepStrictEqual(viaPromise, sync); + assert.deepStrictEqual(viaCallback, sync); + } else { + assert.ok(viaPromise.length > 100 && viaCallback.length > 100, file); + if (file === '/proc/self/maps') assert.ok(viaCallback.length > 4096); + } + } + } + + // async_hooks see the read as an FSREQCALLBACK-typed resource with proper + // init/before/after/destroy, and the callback runs in that context. + { + const seen = new Map(); + const hook = async_hooks.createHook({ + init(id, type) { if (type === 'FSREQCALLBACK') seen.set(id, ['init']); }, + before(id) { seen.get(id)?.push('before'); }, + after(id) { seen.get(id)?.push('after'); }, + destroy(id) { seen.get(id)?.push('destroy'); }, + }).enable(); + await new Promise((res) => fs.readFile(textFile, common.mustSucceed(() => { + assert.ok([...seen.keys()].includes(async_hooks.executionAsyncId())); + res(); + }))); + await new Promise((r) => setImmediate(r)); + hook.disable(); + const complete = [...seen.values()].some((events) => events.join() === 'init,before,after,destroy'); + assert.ok(complete, JSON.stringify([...seen.values()])); + } + + // options.buffer (user-supplied buffer) keeps its own path and semantics. + { + const { file, buf } = files.get(4096); + const target = Buffer.alloc(8192); + const result = await fs.promises.readFile(file, { buffer: target }); + assert.strictEqual(result.buffer, target.buffer); + assert.deepStrictEqual(result, buf); + } +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-trace-events-fs-async.js b/test/parallel/test-trace-events-fs-async.js index 04149dae2e72..9b8e21b3d560 100644 --- a/test/parallel/test-trace-events-fs-async.js +++ b/test/parallel/test-trace-events-fs-async.js @@ -91,7 +91,9 @@ function fdatasync() { function fstat() { const fs = require('fs'); fs.writeFileSync('fs8.txt', '123', 'utf8'); - fs.readFile('fs8.txt', () => { + const fd = fs.openSync('fs8.txt', 'r'); + fs.fstat(fd, () => { + fs.closeSync(fd); fs.unlinkSync('fs8.txt'); }); }