From 21db0ab464d7c3bce8919ebc2b258aa97a5cc03a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 16 Aug 2026 01:08:30 +0000 Subject: [PATCH] fs: read small files in one thread pool round trip fs.readFile(path) took four libuv thread pool round trips for a typical small file -- open, fstat, read and close, each its own uv_fs request with its own queue wait, completion callback and JS/C++ crossing -- and fs.promises.readFile(path) did the same through a FileHandle. For the small files applications read most, the round trips are the cost, and each occupies a slot in the pool that concurrent dns.lookup(), fs and crypto work is also queueing for. Add ReadFileJob (an AsyncWrap + ThreadPoolWork) that performs open + fstat + read-to-EOF + close as one pool task and reports the whole content, or, when the file turns out to be larger than one chunk (kReadFileBufferLength, 512 KiB), stops after fstat() and hands the fd and size back so that the existing chunked reader continues unchanged (large reads stay interleaved and abortable between chunks, and still save the fstat round trip). fs.readFile() and fs.promises.readFile() use it for path arguments without a user buffer; file descriptors, FileHandles, options.buffer and an active VFS keep their paths. Behavior is otherwise kept: same bytes for every size and encoding; open failures report syscall 'open' with the path, read failures 'read'; permission errors are delivered through the callback/promise as before; an abort that arrives while the read is in flight still wins; the job is an FSREQCALLBACK resource for async_hooks; a handed back fd is tracked exactly like one from a plain open(). Tests that asserted the internal open/fstat/read/close request chain, used readFile() as a proxy for an fstat trace event, or injected faults through FileHandle.prototype for path-based reads are adjusted to keep testing what they test (a file just over one chunk where the chain shape matters, fs.fstat() for the fstat trace, a larger file so the FileHandle path is taken). fs.readFile() of 4 KiB files at concurrency 64 goes from ~51k to ~306k files per second, and a mixed stat/readFile/dns.lookup burst from ~66k to ~312k operations per second. Signed-off-by: Shelley Vohr --- lib/fs.js | 64 +++- lib/internal/fs/promises.js | 67 +++- src/node_file.cc | 289 ++++++++++++++++++ .../test-async-exec-resource-match.js | 4 +- .../test-fsreqcallback-readFile.js | 19 +- test/async-hooks/test-graph.fsreq-readFile.js | 11 +- ...s-promises-file-handle-aggregate-errors.js | 5 +- ...st-fs-promises-file-handle-close-errors.js | 5 +- .../test-fs-promises-file-handle-op-errors.js | 5 +- test/parallel/test-fs-promises-readfile.js | 16 +- .../test-fs-readfile-one-roundtrip.js | 152 +++++++++ test/parallel/test-trace-events-fs-async.js | 4 +- 12 files changed, 611 insertions(+), 30 deletions(-) create mode 100644 test/parallel/test-fs-readfile-one-roundtrip.js 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'); }); }