disposable temporary directory

[bakkot]

What is the problem this feature will solve?

I often find myself wanting to make a temporary directory for use during execution of a single function, for example because I'm going to execSync some process which creates temporary files in the current directory. mkdtemp works great to create the thing, but then I have to wrap in in a try/finally to handle cleanup.

What is the feature you are proposing to solve the problem?

This seems like a good case for the fancy new using syntax. It would have to return an object with a [Symbol.dispose] and a path property instead of just a string, but then instead of

let tempDir;
try {
  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'prefix-'));
  spawnSync(command, [], { cwd: tempDir });
  // etc
} finally {
  fs.rmSync(tempDir, { recursive: true, force: true });
}

we could have

using tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'prefix-'), { disposable: true }); // or whatever
spawnSync(command, [], { cwd: tempDir.path });
// cleanup handled automatically

Compare Python's tempfile module, which you can use as a context manager for the same sort of behavior:

# create a temporary directory using the context manager
with tempfile.TemporaryDirectory() as tmpdirname:
    print('created temporary directory', tmpdirname)
# directory and contents have been removed

For bonus points it would be nice if this got cleaned up on process exit, but that's more work and it seems fine to just leave "might not get cleaned up if the process exits in the middle of your function" as a documented behavior.

What alternatives have you considered?

We can of course just keep using try/finally, or not cleaning up.

[juanarbol]

This is such an interesting feature request.

I think I made it work in a "sync fashion"

diff --git a/src/node_file.cc b/src/node_file.cc
index ba8a1c464d..8a00502ca8 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -3125,6 +3125,19 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
     if (is_uv_error(result)) {
       return;
     }
+    auto cleanup = OnScopeLeave([&req_wrap_sync]() {
+      // Delete the folder created by uv_fs_mkdtemp
+      uv_fs_t req;
+      FS_SYNC_TRACE_BEGIN(rmdir);
+      int rmdir_result = uv_fs_rmdir(nullptr, &req, req_wrap_sync.req.path,
+                                     nullptr);
+      FS_SYNC_TRACE_END(rmdir);
+      if (is_uv_error(rmdir_result)) {
+        // If rmdir fails, we don't throw an error here, but we should log it.
+        fprintf(stderr, "Failed to remove temporary directory: %s\n",
+                uv_strerror(rmdir_result));
+      }
+    });
     Local<Value> ret;
     if (StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding)
             .ToLocal(&ret)) {

It just works, now I have to figure things out for the async way. I don't think we can use the using keyword, I would not trust such thing to a "non stable" API.

But that is useless. It just creates it and delete it immediately. Not quite sure how we could tie the "parent's context to the callee"

[bakkot]

I would not trust such thing to a "non stable" API.

It's shipping in Chrome, it should be fine to rely on. The only remaining "instability" you should expect is in dumb edge cases like this.

[juanarbol]

Then I'm not quite sure if that's something we could backport to previous release lines.

[bakkot]

Well, technically you can because implementing a disposable doesn't require use of any syntax (though you might have to polyfill Symbol.dispose).

And if there's a string-named alias (say .remove()) for the Symbol.dispose methods), it's still usable in that you can do

let tempDir;
try {
  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'prefix-'), { disposable: true });
  spawnSync(command, [], { cwd: tempDir.path });
  // etc
} finally {
  tempDir.remove();
}

But it is mostly something you'd want to use with the new syntax.

[juanarbol]

I think you have a clearer picture about the implementation, would you give it a try? Sadly rn I don't have the bandwidth.

[jasnell]

+1 on this, it's certainly an interesting use case

[bakkot]

Draft PR for discussion: #58516

I had some questions there if anyone wants to take a look.