From 8b04b74abffc56770d31d91f60d49b8fcb3e22ce Mon Sep 17 00:00:00 2001
From: Trevor Norris <trev.norris@gmail.com>
Date: Mon, 27 Nov 2023 14:18:44 -0700
Subject: [PATCH] src: change how fd counter is done

Because of a recent change in Node.js it is necessary to move where
open/close fd's are tracked. Also update the test to check additional
cases, and remove the check for GC close cleanup since it's deprecated.
---
 src/node_file-inl.h                           |  6 --
 src/node_file.cc                              |  3 +
 .../parallel/test-nsolid-file-handle-count.js | 75 ++++++++++---------
 3 files changed, 41 insertions(+), 43 deletions(-)

diff --git a/src/node_file-inl.h b/src/node_file-inl.h
index c7392601654..cab8722b6d8 100644
--- a/src/node_file-inl.h
+++ b/src/node_file-inl.h
@@ -352,12 +352,6 @@ int SyncCall(Environment* env, v8::Local<v8::Value> ctx,
     ctx_obj->Set(context,
                  env->syscall_string(),
                  OneByteString(isolate, syscall)).Check();
-  } else {
-    if (strncmp(syscall, "open", 4) == 0) {
-      env->envinst_->inc_fs_handles_opened();
-    } else if (strncmp(syscall, "close", 5) == 0) {
-      env->envinst_->inc_fs_handles_closed();
-    }
   }
   return err;
 }
diff --git a/src/node_file.cc b/src/node_file.cc
index 28bbac958ee..f55849f1a5b 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -1020,6 +1020,7 @@ void Close(const FunctionCallbackInfo<Value>& args) {
     FS_SYNC_TRACE_BEGIN(close);
     SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_close, fd);
     FS_SYNC_TRACE_END(close);
+    env->envinst_->inc_fs_handles_closed();
   }
 }
 
@@ -2038,6 +2039,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
     FS_SYNC_TRACE_END(open);
     if (is_uv_error(result)) return;
     env->AddUnmanagedFd(result);
+    env->envinst_->inc_fs_handles_opened();
     args.GetReturnValue().Set(result);
   }
 }
@@ -2079,6 +2081,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
     }
     FileHandle* fd = FileHandle::New(binding_data, result);
     if (fd == nullptr) return;
+    env->envinst_->inc_fs_handles_opened();
     args.GetReturnValue().Set(fd->object());
   }
 }
diff --git a/test/parallel/test-nsolid-file-handle-count.js b/test/parallel/test-nsolid-file-handle-count.js
index cc2742cdc55..5fb1330483d 100644
--- a/test/parallel/test-nsolid-file-handle-count.js
+++ b/test/parallel/test-nsolid-file-handle-count.js
@@ -9,6 +9,9 @@ const fs = require('fs');
 const baseOpened = nsolid.metrics().fsHandlesOpenedCount;
 const baseClosed = nsolid.metrics().fsHandlesClosedCount;
 
+let oCntr = 0;
+let cCntr = 0;
+
 function getOpened() {
   return nsolid.metrics().fsHandlesOpenedCount - baseOpened;
 }
@@ -23,54 +26,52 @@ try {
   // It's meant to throw and the exception is to be ignored.
 }
 
-assert.strictEqual(getOpened(), 0);
-assert.strictEqual(getClosed(), 0);
+assert.strictEqual(getOpened(), oCntr);
+assert.strictEqual(getClosed(), cCntr);
 
 const fd = fs.openSync(__filename);
-assert.strictEqual(getOpened(), 1);
-assert.strictEqual(getClosed(), 0);
+assert.strictEqual(getOpened(), ++oCntr);
+assert.strictEqual(getClosed(), cCntr);
 
 fs.closeSync(fd);
-assert.strictEqual(getOpened(), 1);
-assert.strictEqual(getClosed(), 1);
+assert.strictEqual(getOpened(), oCntr);
+assert.strictEqual(getClosed(), ++cCntr);
+
+fs.readFileSync(__filename);
+assert.strictEqual(getOpened(), ++oCntr);
+assert.strictEqual(getClosed(), ++cCntr);
 
-fs.open(__filename, common.mustCall((err, fd) => {
-  assert.ok(!err);
-  assert.strictEqual(getOpened(), 2);
-  assert.strictEqual(getClosed(), 1);
+fs.readFile(__filename, () => {
+  assert.strictEqual(getOpened(), ++oCntr);
+  assert.strictEqual(getClosed(), ++cCntr);
 
-  fs.close(fd, common.mustCall((err) => {
+  fs.open(__filename, common.mustCall((err, fd) => {
     assert.ok(!err);
-    assert.strictEqual(getOpened(), 2);
-    assert.strictEqual(getClosed(), 2);
-
-    checkPromise().then(common.mustCall(() => {
-      openFileHandle();
-      setTimeout(() => {
-        // The FileHandle should be out-of-scope and no longer accessed now.
-        global.gc();
-        setImmediate(() => {
-          assert.strictEqual(getOpened(), 4);
-          assert.strictEqual(getClosed(), 4);
-        });
-      }, 100);
-    })).catch(common.mustNotCall());
+    assert.strictEqual(getOpened(), ++oCntr);
+    assert.strictEqual(getClosed(), cCntr);
+
+    fs.close(fd, common.mustCall((err) => {
+      assert.ok(!err);
+      assert.strictEqual(getOpened(), oCntr);
+      assert.strictEqual(getClosed(), ++cCntr);
+
+      checkPromise()
+        .then(common.mustCall(closePromiseFd))
+        .catch(common.mustNotCall());
+    }));
   }));
-}));
+});
 
 async function checkPromise() {
   let fh = await fs.promises.open(__filename);
-  assert.strictEqual(getOpened(), 3);
-  assert.strictEqual(getClosed(), 2);
-  fh = await fh.close();
-  assert.strictEqual(fh, undefined);
-  assert.strictEqual(getOpened(), 3);
-  assert.strictEqual(getClosed(), 3);
+  assert.strictEqual(getOpened(), ++oCntr);
+  assert.strictEqual(getClosed(), cCntr);
+  return fh;
 }
 
-async function openFileHandle() {
-  const fh = await fs.promises.open(__filename);
-  console.log(fh);
-  assert.strictEqual(getOpened(), 4);
-  assert.strictEqual(getClosed(), 3);
+async function closePromiseFd(fh) {
+  fh = await fh.close();
+  assert.strictEqual(fh, undefined);
+  assert.strictEqual(getOpened(), oCntr);
+  assert.strictEqual(getClosed(), ++cCntr);
 }