Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
24 changes: 19 additions & 5 deletions src/node_dir.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
42 changes: 42 additions & 0 deletions test/parallel/test-fs-dir-close-gc-deprecation.mjs
Original file line number Diff line number Diff line change
@@ -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());
});
}));
}
Loading