fs: cancel in-flight stat on abort

Author: mertcanaltin

lib/fs.js

@@ -136,6 +136,7 @@ const {
 const {
   isInt32,
   parseFileMode,
+  validateAbortSignal,
   validateBoolean,
   validateBuffer,
   validateEncoding,
@@ -340,6 +341,26 @@ function checkAborted(signal, callback) {
   return false;
 }
 
+function bindSignalToReq(req, signal, callback) {
+  if (!signal) {
+    req.oncomplete = callback;
+    return;
+  }
+  let aborted = false;
+  const onAbort = () => {
+    aborted = true;
+    req.cancel();
+    callback(new AbortError(undefined, { cause: signal.reason }));
+  };
+  kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
+  signal.addEventListener('abort', onAbort, { __proto__: null, [kResistStopPropagation]: true });
+  req.oncomplete = function(err, result) {
+    signal.removeEventListener('abort', onAbort);
+    if (aborted) return;
+    callback(err, result);
+  };
+}
+
 /**
  * Asynchronously reads the entire contents of a file.
  * @param {string | Buffer | URL | number} path
@@ -1635,11 +1656,12 @@ function stat(path, options = { bigint: false, throwIfNoEntry: true }, callback)
   callback = makeStatsCallback(callback);
   path = getValidatedPath(path);
 
+  if (options.signal !== undefined) validateAbortSignal(options.signal, 'options.signal');
   if (checkAborted(options.signal, callback)) return;
 
   const req = new FSReqCallback(options.bigint);
-  req.oncomplete = callback;
-  binding.stat(getValidatedPath(path), options.bigint, req, options.throwIfNoEntry);
+  bindSignalToReq(req, options.signal, callback);
+  binding.stat(path, options.bigint, req, options.throwIfNoEntry);
 }
 
 function statfs(path, options = { bigint: false }, callback) {

src/node_file.cc

@@ -750,6 +750,12 @@ void NewFSReqCallback(const FunctionCallbackInfo<Value>& args) {
   new FSReqCallback(binding_data, args.This(), args[0]->IsTrue());
 }
 
+void CancelFSReq(const FunctionCallbackInfo<Value>& args) {
+  FSReqBase* req_wrap;
+  ASSIGN_OR_RETURN_UNWRAP(&req_wrap, args.This());
+  req_wrap->Cancel();
+}
+
 FSReqAfterScope::FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req)
     : wrap_(wrap),
       req_(req),
@@ -4215,6 +4221,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   fst->InstanceTemplate()->SetInternalFieldCount(
       FSReqBase::kInternalFieldCount);
   fst->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
+  SetProtoMethod(isolate, fst, "cancel", CancelFSReq);
   SetConstructorFunction(isolate, target, "FSReqCallback", fst);
 
   // Create FunctionTemplate for FileHandleReadWrap. There’s no need
@@ -4326,6 +4333,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 
   registry->Register(Mkdtemp);
   registry->Register(NewFSReqCallback);
+  registry->Register(CancelFSReq);
 
   registry->Register(FileHandle::New);
   registry->Register(FileHandle::Close);

test/parallel/test-fs-stat-abort-test.js

@@ -6,29 +6,34 @@ const assert = require('node:assert');
 const fs = require('node:fs');
 const tmpdir = require('../common/tmpdir');
 
-test('fs.stat should throw AbortError when called with an already aborted AbortSignal', async () => {
-  // This test verifies that fs.stat immediately throws an AbortError if the provided AbortSignal
-  // has already been canceled. This approach is used because trying to abort an fs.stat call in flight
-  // is unreliable given that file system operations tend to complete very quickly on many platforms.
-  tmpdir.refresh();
+tmpdir.refresh();
+const filePath = tmpdir.resolve('temp.txt');
+fs.writeFileSync(filePath, 'Test');
 
-  const filePath = tmpdir.resolve('temp.txt');
-  fs.writeFileSync(filePath, 'Test');
-
-  // Create an already aborted AbortSignal.
+test('fs.stat aborts when signal is already aborted', async () => {
   const signal = AbortSignal.abort();
-
   const { promise, resolve, reject } = Promise.withResolvers();
   fs.stat(filePath, { signal }, (err, stats) => {
-    if (err) {
-      return reject(err);
-    }
+    if (err) return reject(err);
     resolve(stats);
   });
+  await assert.rejects(promise, { name: 'AbortError' });
+});
 
-  // Assert that the promise is rejected with an AbortError.
+test('fs.stat aborts in-flight when signal aborts after the call', async () => {
+  const controller = new AbortController();
+  const { promise, resolve, reject } = Promise.withResolvers();
+  fs.stat(filePath, { signal: controller.signal }, (err, stats) => {
+    if (err) return reject(err);
+    resolve(stats);
+  });
+  controller.abort();
   await assert.rejects(promise, { name: 'AbortError' });
+});
 
-  fs.unlinkSync(filePath);
-  tmpdir.refresh();
+test('fs.stat throws ERR_INVALID_ARG_TYPE for invalid signal', () => {
+  assert.throws(
+    () => fs.stat(filePath, { signal: 'not-a-signal' }, () => {}),
+    { code: 'ERR_INVALID_ARG_TYPE' },
+  );
 });