src: unlink temporary compile cache file on persist failure

src/compile_cache.cc

@@ -476,6 +476,30 @@ void CompileCacheHandler::Persist() {
       continue;
     }
     Debug(" -> %s\n", mkstemp_req.path);
+
+    // From this point on, the temporary file exists on disk and its
+    // descriptor (mkstemp_req.result) is open. If persistence does not
+    // complete (write, close, or rename fails), make sure the descriptor is
+    // closed and the temporary file is removed instead of being leaked.
+    // Otherwise a persistent failure (e.g. ENOSPC/EDQUOT/EFBIG) can
+    // accumulate zero-byte temporary files and open file descriptors
+    // indefinitely.
+    bool fd_open = true;
+    bool renamed = false;
+    std::string mkstemp_path = mkstemp_req.path;
+    auto cleanup_tmp_file = OnScopeLeave([&]() {
+      if (fd_open) {
+        uv_fs_t close_req;
+        uv_fs_close(nullptr, &close_req, mkstemp_req.result, nullptr);
+        uv_fs_req_cleanup(&close_req);
+      }
+      if (!renamed) {
+        uv_fs_t unlink_req;
+        uv_fs_unlink(nullptr, &unlink_req, mkstemp_path.c_str(), nullptr);
+        uv_fs_req_cleanup(&unlink_req);
+      }
+    });
+
     Debug("[compile cache] writing cache for %s %s to temporary file %s [%d "
           "%d %d "
           "%d %d]...",
@@ -508,6 +532,10 @@ void CompileCacheHandler::Persist() {
     auto cleanup_close =
         OnScopeLeave([&close_req]() { uv_fs_req_cleanup(&close_req); });
     err = uv_fs_close(nullptr, &close_req, mkstemp_req.result, nullptr);
+    // The descriptor is no longer usable after uv_fs_close() is attempted,
+    // regardless of whether it succeeded, so the scope guard above should
+    // not try to close it again.
+    fd_open = false;
 
     if (err < 0) {
       Debug("failed: %s\n", uv_strerror(err));
@@ -533,6 +561,9 @@ void CompileCacheHandler::Persist() {
       Debug("failed: %s\n", uv_strerror(err));
       continue;
     }
+    // The temporary file no longer exists at mkstemp_path; nothing left for
+    // the scope guard to unlink.
+    renamed = true;
     Debug("success\n");
     entry->persisted = true;
   }

test/parallel/test-compile-cache-persist-write-failure.js

@@ -0,0 +1,80 @@
+'use strict';
+
+// Regression test for https://github.com/nodejs/node/issues/65473
+//
+// When CompileCacheHandler::Persist() fails to write, close, or rename the
+// temporary cache file (e.g. because the underlying filesystem operation
+// fails with ENOSPC/EDQUOT/EFBIG), it used to `continue` without closing the
+// already-open descriptor or removing the already-created temporary file.
+// A persistent failure could therefore leak an open file descriptor and a
+// zero-byte temporary file for every module compiled, for the lifetime of
+// the process.
+//
+// This uses `ulimit -f 0` to make uv_fs_mkstemp() succeed (creating an empty,
+// 0-byte file is allowed) while the subsequent uv_fs_write() fails with
+// EFBIG, reliably reproducing a failure partway through persistence without
+// needing to fill up a real filesystem.
+
+const common = require('../common');
+
+if (common.isWindows)
+  common.skip('no RLIMIT_FSIZE on Windows');
+
+if (process.config.variables.node_shared)
+  common.skip('SIGXFSZ signal handler not installed in shared library mode');
+
+const assert = require('assert');
+const child_process = require('child_process');
+const fixtures = require('../common/fixtures');
+const tmpdir = require('../common/tmpdir');
+const fs = require('fs');
+const path = require('path');
+
+function listFilesRecursive(dir) {
+  const result = [];
+  for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) {
+      result.push(...listFilesRecursive(full));
+    } else {
+      result.push(full);
+    }
+  }
+  return result;
+}
+
+tmpdir.refresh();
+const cacheDir = tmpdir.resolve('.compile_cache_dir');
+
+const [cmd, opts] = common.escapePOSIXShell`ulimit -f 0 && "${process.execPath}" "${fixtures.path('empty.js')}"`;
+const result = child_process.spawnSync('/bin/sh', ['-c', cmd], {
+  ...opts,
+  env: {
+    ...opts.env,
+    NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
+    NODE_COMPILE_CACHE: cacheDir,
+  },
+  cwd: tmpdir.path,
+});
+
+const stderr = result.stderr.toString();
+const stdout = result.stdout.toString();
+
+assert.strictEqual(result.status, 0, `child should exit cleanly, got status ${result.status}\nstderr: ${stderr}`);
+
+// Sanity check: the failure path this test targets was actually exercised.
+// If this does not match, `ulimit -f 0` did not make persistence fail the
+// way this test expects, and the test would pass vacuously.
+assert.match(
+  stderr,
+  /writing cache for .*empty\.js.*failed: /,
+  `expected a failed persistence attempt in stderr, got:\n${stderr}\n---\n${stdout}`);
+
+// The actual regression check: nothing should be left behind in the compile
+// cache directory once the failed child process has exited, no matter how
+// persistence failed.
+const leftover = fs.existsSync(cacheDir) ? listFilesRecursive(cacheDir) : [];
+assert.deepStrictEqual(
+  leftover,
+  [],
+  `expected no leftover files in the compile cache directory, found:\n${leftover.join('\n')}`);