fs: runtime deprecate closing fs.Dir on garbage collection

Author: LiviaMedeiros

doc/api/deprecations.md

@@ -4108,12 +4108,15 @@ an internal nodejs implementation rather than a public facing API, use `node:htt
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/58850
+    description: Runtime deprecation.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/59839
     description: Documentation-only deprecation.
 -->
 
-Type: Documentation-only
+Type: Runtime
 
 Allowing a [`fs.Dir`][] object to be closed on garbage collection is
 deprecated. In the future, doing so might result in a thrown error that will

src/env-inl.h

@@ -919,6 +919,14 @@ inline void Environment::RemoveHeapSnapshotNearHeapLimitCallback(
                                         heap_limit);
 }
 
+bool Environment::dir_gc_close_warning() const {
+  return emit_dir_gc_warning_;
+}
+
+void Environment::set_dir_gc_close_warning(bool on) {
+  emit_dir_gc_warning_ = on;
+}
+
 }  // namespace node
 
 // These two files depend on each other. Including base_object-inl.h after this

src/env.h

@@ -820,6 +820,9 @@ class Environment final : public MemoryRetainer {
   inline node_module* extra_linked_bindings_tail();
   inline const Mutex& extra_linked_bindings_mutex() const;
 
+  inline bool dir_gc_close_warning() const;
+  inline void set_dir_gc_close_warning(bool on);
+
   inline void set_source_maps_enabled(bool on);
   inline bool source_maps_enabled() const;
 
@@ -1103,6 +1106,7 @@ class Environment final : public MemoryRetainer {
   std::shared_ptr<KVStore> env_vars_;
   bool printed_error_ = false;
   bool trace_sync_io_ = false;
+  bool emit_dir_gc_warning_ = true;
   bool emit_env_nonstring_warning_ = true;
   bool emit_err_name_warning_ = true;
   bool source_maps_enabled_ = false;

src/node_dir.cc

@@ -164,13 +164,27 @@ inline void DirHandle::GCClose() {
   }
 
   // If the close was successful, we still want to emit a process warning
-  // to notify that the file descriptor was gc'd. We want to be noisy about
+  // to notify that the directory handle was gc'd. We want to be noisy about
   // this because not explicitly closing the DirHandle is a bug.
 
-  env()->SetImmediate([](Environment* env) {
-    ProcessEmitWarning(env,
-                       "Closing directory handle on garbage collection");
-  }, CallbackFlags::kUnrefed);
+  env()->SetImmediate(
+      [](Environment* env) {
+        ProcessEmitWarning(env,
+                           "Closing directory handle on garbage collection");
+        if (env->dir_gc_close_warning()) {
+          env->set_dir_gc_close_warning(false);
+          USE(ProcessEmitDeprecationWarning(
+              env,
+              "Closing a Dir object on garbage collection is deprecated. "
+              "Please close Dir objects explicitly using "
+              "Dir.prototype.close(), "
+              "or by using explicit resource management ('using' keyword). "
+              "In the future, an error will be thrown if a directory is closed "
+              "during garbage collection.",
+              "DEP0200"));
+        }
+      },
+      CallbackFlags::kUnrefed);
 }
 
 void AfterClose(uv_fs_t* req) {

test/parallel/test-fs-dir-close-gc-deprecation.mjs

@@ -0,0 +1,42 @@
+// Flags: --expose-gc --no-warnings
+
+// Test that a runtime warning is emitted when a Dir object
+// is allowed to close on garbage collection. In the future, this
+// test should verify that closing on garbage collection throws a
+// process fatal exception.
+
+import { expectWarning, mustCall } from '../common/index.mjs';
+import { rejects } from 'node:assert';
+import { opendir } from 'node:fs/promises';
+import { setImmediate } from 'node:timers';
+
+const warning =
+  'Closing a Dir object on garbage collection is deprecated. ' +
+  'Please close Dir objects explicitly using Dir.prototype.close(), ' +
+  "or by using explicit resource management ('using' keyword). " +
+  'In the future, an error will be thrown if a directory is closed during garbage collection.';
+
+// Test that closing Dir automatically using iterator doesn't emit warning
+{
+  const dir = await opendir(import.meta.dirname);
+  await Array.fromAsync(dir);
+  await rejects(dir.close(), { code: 'ERR_DIR_CLOSED' });
+}
+
+{
+  expectWarning({
+    Warning: [['Closing directory handle on garbage collection']],
+    DeprecationWarning: [[warning, 'DEP0200']],
+  });
+
+  await opendir(import.meta.dirname).then(mustCall(() => {
+    setImmediate(() => {
+      // The dir is out of scope now
+      globalThis.gc();
+
+      // Wait an extra event loop turn, as the warning is emitted from the
+      // native layer in an unref()'ed setImmediate() callback.
+      setImmediate(mustCall());
+    });
+  }));
+}