From cf3b252491dc74f4b815f1b4456f9ab99cf3f5f8 Mon Sep 17 00:00:00 2001 From: Samuel Mbabhazi Date: Tue, 13 Oct 2026 05:53:57 +0200 Subject: [PATCH] vfs: route mkdtempDisposable through the fs hooks fs.mkdtempSync() and fsPromises.mkdtemp() consult the VFS handlers, but fs.mkdtempDisposableSync() and fsPromises.mkdtempDisposable() call the binding directly. On a mounted path they fail with ENOTDIR, because the mount point does not exist on the real file system, while the non-disposable variants succeed. Add the two handlers. They create the directory with the VFS mkdtemp and remove it with the VFS rm, so the disposable object behaves like the real one: the path is a Buffer when the buffer encoding is requested, and remove() can be called more than once. Refs: https://github.com/nodejs/node/pull/65852 Signed-off-by: Samuel Mbabhazi --- lib/fs.js | 6 + lib/internal/fs/promises.js | 6 + lib/internal/vfs/setup.js | 29 +++++ .../test-vfs-fs-mkdtemp-disposable.js | 111 ++++++++++++++++++ 4 files changed, 152 insertions(+) create mode 100644 test/parallel/test-vfs-fs-mkdtemp-disposable.js diff --git a/lib/fs.js b/lib/fs.js index 4436fa2df6e6..c79f57537020 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -3750,6 +3750,12 @@ function mkdtempSync(prefix, options) { * @returns {object} A disposable object with a "path" property. */ function mkdtempDisposableSync(prefix, options) { + const h = vfsState.handlers; + if (h !== null) { + const result = h.mkdtempDisposableSync(prefix, options); + if (result !== undefined) return result; + } + options = getOptions(options); if (BufferIsBuffer(prefix)) { options = { ...options, encoding: 'buffer' }; diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index c21b1e5740fc..db348476c35e 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -2137,6 +2137,12 @@ async function mkdtemp(prefix, options) { } async function mkdtempDisposable(prefix, options) { + const h = vfsState.handlers; + if (h !== null) { + const promise = h.mkdtempDisposable(prefix, options); + if (promise !== undefined) return await promise; + } + options = getOptions(options); if (BufferIsBuffer(prefix)) { options = { ...options, encoding: 'buffer' }; diff --git a/lib/internal/vfs/setup.js b/lib/internal/vfs/setup.js index a5f1b42659b0..08aeb99dfa2e 100644 --- a/lib/internal/vfs/setup.js +++ b/lib/internal/vfs/setup.js @@ -9,6 +9,8 @@ const { String, StringPrototypeEndsWith, StringPrototypeStartsWith, + SymbolAsyncDispose, + SymbolDispose, } = primordials; const { Buffer } = require('buffer'); @@ -530,6 +532,19 @@ function createVfsHandlers() { } return result; }, + mkdtempDisposableSync(prefix, options) { + return vfsOp(prefix, (vfs, n) => { + const dir = vfs.mkdtempSync(n); + const remove = () => vfs.rmSync(dir, { recursive: true, force: true }); + return { + path: options?.encoding === 'buffer' ? Buffer.from(dir) : dir, + remove, + [SymbolDispose]() { + remove(); + }, + }; + }); + }, opendirSync: (path, options) => vfsOp(path, (vfs, n) => vfs.opendirSync(n, options)), openAsBlob(path, options) { const pathStr = toPathStr(path); @@ -860,6 +875,20 @@ function createVfsHandlers() { } return promise; }, + mkdtempDisposable(prefix, options) { + return vfsOp(prefix, async (vfs, n) => { + const dir = await vfs.promises.mkdtemp(n); + const remove = () => vfs.promises.rm(dir, { recursive: true, force: true }); + return { + __proto__: null, + path: options?.encoding === 'buffer' ? Buffer.from(dir) : dir, + remove, + async [SymbolAsyncDispose]() { + await remove(); + }, + }; + }); + }, chmod: (path, mode) => vfsOp(path, (vfs, n) => vfs.promises.chmod(n, mode).then(() => true)), utimes: (path, atime, mtime) => diff --git a/test/parallel/test-vfs-fs-mkdtemp-disposable.js b/test/parallel/test-vfs-fs-mkdtemp-disposable.js new file mode 100644 index 000000000000..3314f49ce287 --- /dev/null +++ b/test/parallel/test-vfs-fs-mkdtemp-disposable.js @@ -0,0 +1,111 @@ +// Flags: --experimental-vfs +'use strict'; + +// fs.mkdtempDisposableSync and fs.promises.mkdtempDisposable dispatch to the +// VFS for a mounted prefix, like mkdtempSync and fs.promises.mkdtemp do, and +// their remove() deletes the directory from the VFS. A prefix outside the +// mount still reaches the real file system. + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const vfs = require('node:vfs'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const myVfs = vfs.create(); +myVfs.mkdirSync('/work', { recursive: true }); +const mountPoint = myVfs.mount(); +const prefix = path.join(mountPoint, 'work', 'tmp-'); + +// Sync: created in the VFS, removed from the VFS, removal is idempotent. +{ + const result = fs.mkdtempDisposableSync(prefix); + assert.ok(result.path.startsWith(prefix)); + assert.strictEqual(result.path.length, prefix.length + 6); + assert.strictEqual(fs.statSync(result.path).isDirectory(), true); + assert.deepStrictEqual(fs.readdirSync(path.join(mountPoint, 'work')), [path.basename(result.path)]); + + fs.writeFileSync(path.join(result.path, 'file.txt'), 'data'); + result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); + assert.deepStrictEqual(fs.readdirSync(path.join(mountPoint, 'work')), []); + result.remove(); +} + +// Sync: Symbol.dispose +{ + const result = fs.mkdtempDisposableSync(prefix); + assert.strictEqual(fs.existsSync(result.path), true); + result[Symbol.dispose](); + assert.strictEqual(fs.existsSync(result.path), false); + result[Symbol.dispose](); +} + +// Sync: buffer encoding +{ + const result = fs.mkdtempDisposableSync(prefix, { encoding: 'buffer' }); + assert.ok(Buffer.isBuffer(result.path)); + assert.ok(result.path.toString().startsWith(prefix)); + assert.strictEqual(fs.existsSync(result.path), true); + result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); +} + +// Sync: a prefix outside the mount uses the real file system. +{ + const result = fs.mkdtempDisposableSync(tmpdir.resolve('real-')); + assert.strictEqual(path.dirname(result.path), tmpdir.path); + assert.strictEqual(fs.existsSync(result.path), true); + result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); +} + +(async () => { + // Promises: created in the VFS, removed from the VFS, removal is idempotent. + { + const result = await fs.promises.mkdtempDisposable(prefix); + assert.ok(result.path.startsWith(prefix)); + assert.strictEqual(result.path.length, prefix.length + 6); + assert.strictEqual(fs.statSync(result.path).isDirectory(), true); + assert.deepStrictEqual(fs.readdirSync(path.join(mountPoint, 'work')), [path.basename(result.path)]); + + fs.writeFileSync(path.join(result.path, 'file.txt'), 'data'); + await result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); + assert.deepStrictEqual(fs.readdirSync(path.join(mountPoint, 'work')), []); + await result.remove(); + } + + // Promises: Symbol.asyncDispose + { + const result = await fs.promises.mkdtempDisposable(prefix); + assert.strictEqual(fs.existsSync(result.path), true); + await result[Symbol.asyncDispose](); + assert.strictEqual(fs.existsSync(result.path), false); + await result[Symbol.asyncDispose](); + } + + // Promises: buffer encoding + { + const result = await fs.promises.mkdtempDisposable(prefix, { encoding: 'buffer' }); + assert.ok(Buffer.isBuffer(result.path)); + assert.ok(result.path.toString().startsWith(prefix)); + assert.strictEqual(fs.existsSync(result.path), true); + await result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); + } + + // Promises: a prefix outside the mount uses the real file system. + { + const result = await fs.promises.mkdtempDisposable(tmpdir.resolve('real-')); + assert.strictEqual(path.dirname(result.path), tmpdir.path); + assert.strictEqual(fs.existsSync(result.path), true); + await result.remove(); + assert.strictEqual(fs.existsSync(result.path), false); + } + + myVfs.unmount(); +})().then(common.mustCall());