Skip to content

Commit 96c78d7

Browse files
Renegade334RafaelGSS
authored andcommitted
fs: make Dir disposers idempotent
PR-URL: #58692 Refs: #58206 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent fa6854f commit 96c78d7

File tree

3 files changed

+21
-24
lines changed

3 files changed

+21
-24
lines changed

doc/api/fs.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6769,8 +6769,8 @@ changes:
67696769
description: No longer experimental.
67706770
-->
67716771
6772-
Calls `dir.close()` and returns a promise that fulfills when the
6773-
dir is closed.
6772+
Calls `dir.close()` if the directory handle is open, and returns a promise that
6773+
fulfills when disposal is complete.
67746774
67756775
#### `dir[Symbol.dispose]()`
67766776
@@ -6782,7 +6782,8 @@ changes:
67826782
description: No longer experimental.
67836783
-->
67846784
6785-
Calls `dir.closeSync()` and returns `undefined`.
6785+
Calls `dir.closeSync()` if the directory handle is open, and returns
6786+
`undefined`.
67866787
67876788
### Class: `fs.Dirent`
67886789

lib/internal/fs/dir.js

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ const {
2424

2525
const { FSReqCallback } = binding;
2626
const {
27-
assignFunctionName,
2827
promisify,
2928
} = require('internal/util');
3029
const {
@@ -296,31 +295,24 @@ class Dir {
296295
await this.#closePromisified();
297296
}
298297
}
298+
299+
[SymbolDispose]() {
300+
if (this.#closed) return;
301+
this.closeSync();
302+
}
303+
304+
async [SymbolAsyncDispose]() {
305+
if (this.#closed) return;
306+
await this.#closePromisified();
307+
}
299308
}
300309

301-
const nonEnumerableDescriptor = {
302-
enumerable: false,
303-
writable: true,
304-
configurable: true,
305-
};
306310
ObjectDefineProperties(Dir.prototype, {
307-
[SymbolDispose]: {
308-
__proto__: null,
309-
...nonEnumerableDescriptor,
310-
value: assignFunctionName(SymbolDispose, function() {
311-
this.closeSync();
312-
}),
313-
},
314-
[SymbolAsyncDispose]: {
315-
__proto__: null,
316-
...nonEnumerableDescriptor,
317-
value: assignFunctionName(SymbolAsyncDispose, function() {
318-
this.close();
319-
}),
320-
},
321311
[SymbolAsyncIterator]: {
322312
__proto__: null,
323-
...nonEnumerableDescriptor,
313+
enumerable: false,
314+
writable: true,
315+
configurable: true,
324316
value: Dir.prototype.entries,
325317
},
326318
});

test/parallel/test-fs-promises-file-handle-dispose.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@ async function explicitCall() {
1111

1212
const dh = await fs.opendir(__dirname);
1313
await dh[Symbol.asyncDispose]();
14+
// Repeat invocations should not reject
15+
await dh[Symbol.asyncDispose]();
1416
await assert.rejects(dh.read(), { code: 'ERR_DIR_CLOSED' });
1517

1618
const dhSync = opendirSync(__dirname);
1719
dhSync[Symbol.dispose]();
20+
// Repeat invocations should not throw
21+
dhSync[Symbol.dispose]();
1822
assert.throws(() => dhSync.readSync(), { code: 'ERR_DIR_CLOSED' });
1923
}
2024

0 commit comments

Comments
 (0)