Skip to content

Commit ce546e4

Browse files
authored
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 88f4cef commit ce546e4

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
@@ -6773,8 +6773,8 @@ changes:
67736773
description: No longer experimental.
67746774
-->
67756775
6776-
Calls `dir.close()` and returns a promise that fulfills when the
6777-
dir is closed.
6776+
Calls `dir.close()` if the directory handle is open, and returns a promise that
6777+
fulfills when disposal is complete.
67786778
67796779
#### `dir[Symbol.dispose]()`
67806780
@@ -6786,7 +6786,8 @@ changes:
67866786
description: No longer experimental.
67876787
-->
67886788
6789-
Calls `dir.closeSync()` and returns `undefined`.
6789+
Calls `dir.closeSync()` if the directory handle is open, and returns
6790+
`undefined`.
67906791
67916792
### Class: `fs.Dirent`
67926793

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)