From 2e2dbae9ed84527a51212933d8c81564799d2d63 Mon Sep 17 00:00:00 2001
From: hmache <hmache@outlook.com>
Date: Fri, 25 Sep 2026 11:17:09 +0000
Subject: [PATCH] src: unlink temporary compile cache file on persist failure

CompileCacheHandler::Persist() writes each compile cache entry to a
freshly created temporary file (via uv_fs_mkstemp()) and then renames
it to the final cache filename. When the write, close, or rename step
fails, the loop body ran `continue` without closing the still-open
file descriptor or removing the already-created temporary file.

Under a persistent failure condition (ENOSPC, EDQUOT, EFBIG, or any
other error once the temporary file exists), this leaked a zero-byte
temporary file and an open file descriptor for every module Node
tried to cache, for the remaining lifetime of the process. In one
reported EDQUOT incident, a single process left 3,424 zero-byte
compile-cache temporary files behind in one minute, turning a block
quota failure into an inode exhaustion problem.

Add a scope guard, armed right after uv_fs_mkstemp() succeeds, that
closes the descriptor if it is still open and unlinks the temporary
file unless the rename to the final name has already succeeded. The
guard runs on every `continue` in the loop body (including the ones
already present for the write/close/rename failure paths), so no
extra failure handling is needed at each call site beyond marking the
descriptor closed after the close attempt and marking the file
renamed after a successful rename. The original persistence error is
preserved: only the leaked resources are cleaned up, and Persist()
continues to `continue` past the failed entry exactly as before.

A regression test spawns a child process under `ulimit -f 0`, which
lets uv_fs_mkstemp() create an empty (0-byte) temporary file but
makes the subsequent uv_fs_write() fail with EFBIG. This reproduces a
failure partway through persistence deterministically, without
needing to fill up a real filesystem, and the test asserts that no
file is left behind in the compile cache directory afterward.

Fixes: https://github.com/nodejs/node/issues/65473

Signed-off-by: hmache <hmache@outlook.com>
Assisted-by: Claude
---
 src/compile_cache.cc                          | 31 +++++++
 ...est-compile-cache-persist-write-failure.js | 80 +++++++++++++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 test/parallel/test-compile-cache-persist-write-failure.js

diff --git a/src/compile_cache.cc b/src/compile_cache.cc
index 6e891619f42..a6f5df944f8 100644
--- a/src/compile_cache.cc
+++ b/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;
   }
diff --git a/test/parallel/test-compile-cache-persist-write-failure.js b/test/parallel/test-compile-cache-persist-write-failure.js
new file mode 100644
index 00000000000..5ad40d19e55
--- /dev/null
+++ b/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')}`);