FSDirectory: a pending-delete retry on one thread can delete a file that another thread has just re-created under the same name

On a file system that refuses to delete a file while it is open (in practice Windows, with a memory-mapped file or a virus scanner holding a handle), FSDirectory.deleteFile records the name as a pending delete, hides it from listAll(), and retries the delete from a later mutating call on any thread (the call at which a shared call counter reaches the number of pending names). The retry, deletePendingFiles(), is synchronized and deletes by name every entry of a copy of the pending set. createOutput and rename may re-use a pending name ("If this file was pending delete, we are now bringing it back to life"): they take the name off the set, delete the old file and create or move the new one, without that monitor. So a retry that copied the set before the revive deletes the new file afterwards, although every call that wrote it succeeded: in the one-test repro listAll() returns [] instead of [x], and a diagnostic run of the same steps shows openInput("x") throwing NoSuchFileException. The other thread's call is unrelated (syncMetaData()), so no per-name application lock avoids it. It has been there since pending deletes arrived in 6.0.0 (LUCENE-6835, 2016) and fails on 10.6.0 and main. IndexWriter never re-uses a pending name, so the exposure is application or downstream code that uses the Directory directly. Making the revive take the same monitor (about ten lines) fixes it; the repro and all 8,898 core tests pass with it. Everything ran under the test framework's WindowsFS emulation on Linux, not on real Windows.

// FSDirectory.java:207-216  createOutput
public IndexOutput createOutput(String name, IOContext context) throws IOException {
  ensureOpen();
  maybeDeletePendingFiles();                      // :209  may run a retry (synchronized) itself
  // If this file was pending delete, we are now bringing it back to life:
  if (pendingDeletes.remove(name)) {              // :211  no monitor held
    privateDeleteFile(name, true); // try again to delete it - this is the best effort
    pendingDeletes.remove(name); // watch out - if the delete fails it put
  }
  return new FSIndexOutput(name);                 // :215  WRITE, CREATE_NEW
}

// FSDirectory.java:312-323
public synchronized void deletePendingFiles() throws IOException {
  if (pendingDeletes.isEmpty() == false) {
    // Clone the set since we mutate it in privateDeleteFile:
    for (String name : new HashSet<>(pendingDeletes)) {   // :319  a copy
      privateDeleteFile(name, true);                     // Files.delete by name (:339)
    }
  }
}

1. A name becomes pending. privateDeleteFile (:337-368) calls Files.delete; any IOException other than "no such file" puts the name in pendingDeletes (:355-366: "On windows, a file delete can fail because there's still an open file handle against it"). listAll() skips pending names (:172-195) and openInput refuses them (ensureCanRead, :236-241).
2. Any thread runs the retry. createOutput (:209), createTempOutput (:222), sync (:250), rename (:260), syncMetaData (:273) and deleteFile (:305) call maybeDeletePendingFiles (:325-335), which counts the call in a shared AtomicInteger (:99) and, when the count reaches the current pendingDeletes.size(), resets it and runs deletePendingFiles() (:329-332). With a fixed set of N pending names that is every N-th call; names added or removed and concurrent calls shift it.
3. The retry works on a copy. It holds the FSDirectory monitor and deletes, by name, every entry of new HashSet<>(pendingDeletes) (:319), without checking whether a name is still pending.
4. The revive does not take the monitor. createOutput (:211-214) and rename (:261-264) remove the name, retry the old file's delete, then create (:215) or move (:265) the new file. Nothing waits for a retry that is already running.
5. The repro's interleaving. deleteFile("x") and deleteFile("y") are refused; x's reader closes. Thread A's syncMetaData() reaches the counter threshold (2 calls for 2 pending names) and retries from the copy {x, y}. At the retry's first delete a test hook starts thread B and waits until B has finished or is blocked. B's createOutput("x") (its own counter is 1 of 2, so it runs no retry) removes x from the set, deletes the old x, creates the new one and closes it. A resumes and Files.delete(x) removes the new file; y is refused again. The hook only fixes the order of the two threads; it changes no directory or file state.
6. Without any hook. A stress test with ordinary scheduling (64 held files, 2,000 re-creates while another thread keeps calling sync) lost 199 to 321 of 2,000 re-created files on main in three runs, and 0 with the fix. Its op rate is deliberately aggressive; these are not field rates.
7. Secondary symptom, emulation only. If A's retry reaches the name after B deleted the old file and before B created the new one, Files.delete throws NoSuchFileException, which privateDeleteFile rethrows when not on Windows (:341-354), so A's unrelated call fails (406 times in one stress run). On real Windows that branch swallows this exception (by reading, not run on Windows), so there the symptom is the lost file alone.

• Who is hit. Applications or downstream code that use an FSDirectory (MMapDirectory, the FSDirectory.open default, or NIOFSDirectory) directly, with default settings, on a file system that refuses to delete open files, and that re-create (createOutput) or rename onto a name they deleted while it was still open, while another thread makes any mutating call on the same directory. No scale is needed.
• What goes wrong. A file whose createOutput (or rename) and close succeeded silently disappears. Nothing fails at write time; the loss shows up later as NoSuchFileException or as a file missing from listAll().
• Which file systems. In practice Windows, where a memory-mapped file or an outside handle (a virus scanner, a backup tool) makes the delete fail; the code comments also name "a Linux box with a CIFS mount". By reading, not run on Windows: Java opens files on Windows with delete sharing by default, so NIOFSDirectory's own readers probably do not block a delete there, while memory-mapped files do. The repro's NIOFSDirectory on WindowsFS emulates a delete-refusing file system.
• What does not go wrong. No in-tree caller is known to re-use a pending name, so IndexWriter-managed index files are not known to be exposed: IndexFileDeleter counts getPendingDeletions() before it inflates generations, so file generations move past pending names, and the NRT ReplicaNode refuses to start while deletions are pending. The replicator copies into temp files (createTempOutput skips pending names) and then renames each onto its final name (CopyJob.finish), which is this bug's rename path; those final names are new segment file names, so it is not known to rename onto a pending name either.
• How likely: rare. The window is narrow (the stress test lost 0 of 2,000 when the name was pending only briefly) and nothing has been reported in about ten years. Impact class data corruption (a successfully written file is deleted); priority low.

Why two pending names: the retry runs when a call counter reaches the number of pending names. With x alone that threshold is 1, so B's own createOutput would run the retry itself and block on the monitor A holds; y raises it to 2 and keeps B's call (counter 1) below it. With a fix that serializes the revive, B is blocked when the hook lets the retry go on, so the test passes instead of hanging.

Deterministic: 10 of 10 failures with 10 iterations, also with one CPU; the seed only sets the test framework's randomization. The same failure through the repro harness on main (assertions on and off), 10.6.0 (on and off) and 10.5.2. The follow-up's history probe, with a rename scenario and a sequential control that keeps the file, loses the file on every release run from 6.0.0 to 10.5.2 (on 6.2.0-7.3.1 only through rename). Use this one test as the regression test too (its hook spins without a timeout; an upstream version may prefer a bounded wait that fails with a message). Everything ran under WindowsFS emulation on Linux, not on real Windows.

# gradle, main 645d4fe14a20, seed DEADBEEF, assertions on
TestFSDirectoryPendingDeleteRetry > testRetryKeepsFileRecreatedByAnotherThread FAILED
    java.lang.AssertionError: expected x in listAll() but was []
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:D230F9D95B40624C]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.apache.lucene.store.TestFSDirectoryPendingDeleteRetry.testRetryKeepsFileRecreatedByAnotherThread(TestFSDirectoryPendingDeleteRetry.java:71)

# repro harness, the same message: main -ea and -da, 10.6.0 -ea and -da, 10.5.2 -ea (1 run, 1 failed each)
# diagnostic copy of the test (not the repro), main: x is not on disk, openInput("x") throws NoSuchFileException
# with fix A or fix B: the test passes (gradle; harness -ea and -da)

• Contract. Directory.java:35-40: "A file in a directory can be created (createOutput), appended to, then closed. ... Once a file is created it must only be opened for input (openInput), or deleted (deleteFile)." Directory.java:83-84: createOutput "Creates a new, empty file in the directory".
• The revive is deliberate. FSDirectory.java:210: "If this file was pending delete, we are now bringing it back to life". Since LUCENE-8275 (#9322, 7.4.0) it even deletes the old file first so that the new one can be created.
• Precedent. LUCENE-8318 (#9365, 2018) fixed exactly this symptom, "This causes the dest file to be deleted later by deletePendingFiles", for a single-threaded cause (a failed retry put the name back). Its test asserted that the renamed-onto file survives (testRenameWithPendingDeletes, removed together with SimpleFSDirectory in 2020).
• The code is built for concurrent callers. The pending set is a concurrent set (FSDirectory.java:97), the call counter an AtomicInteger (:99), the retry is synchronized, and any thread's call may run it; #11619 (LUCENE-10583) shows retries running on merge threads in production. This is the implementation, not an API promise: the Directory javadoc does not say that concurrent mutation of one directory is safe, so a maintainer may still call this use unsupported.
• History. None of the LUCENE-6835, LUCENE-8275 or LUCENE-8318 discussions mentions a re-create racing a retry on another thread; no "by design" or "won't fix" statement covers it.

Strongest case for "intended": Lucene is write-once; IndexWriter never re-uses a pending name (LUCENE-8310 made generations move past them); the Directory javadoc promises nothing about concurrent use of one name; and the pending-delete layer is openly best effort ("this is hacky/lenient", FSDirectory.java:360; "this is the best effort", :212). So: do not re-create names on Windows.

Why it fails:

1. The application does not use one name concurrently: thread A only calls syncMetaData() (or sync() of another file). The delete of x is FSDirectory's own retry, which no per-name application lock can avoid.
2. "Best effort" describes the retried delete of the old file. Nothing accepts losing the new, successfully closed file, and LUCENE-8318 treated that loss as a bug.
3. FSDirectory implements the revive on purpose. What IndexWriter does limits who is hit, not whether it is a bug.

Verdict: not intended, a defect with narrow reach. A likely objection is that names are not re-used and that pending deletes are rare outside Windows (a reviewer rated the single-threaded form of this pattern in a downstream FSDirectory subclass "Low severity since names aren't reused and this is Linux in practice", elastic/elasticsearch#155919); the points above answer it.

Two code fixes were run; A is recommended, B is the hunt's simpler variant. Both make a retry that already copied the pending set finish before the name is revived, and neither adds a lock object: createOutput already enters this monitor whenever its own maybeDeletePendingFiles runs a retry (FSDirectory.java:329-332). Workaround for users today: do not re-create or rename onto a name you deleted while it might still be open; use a fresh name, as IndexWriter does, or serialize all calls on that directory with your own lock (not the Directory instance, as Directory.java:43-45 warns).

Move the revive block of createOutput and rename into a private synchronized helper and call it only when pendingDeletes.contains(name). The unlocked check is safe for this race: a retry holds the monitor from its copy to the end of its loop, and during that loop a name in its copy leaves the set only through the retry itself (which is then done with it) or a revive (which now needs the monitor). With no pending deletes, the normal case on POSIX local file systems, createOutput and rename behave as today and still take no lock; the only change there is a contains lookup on the empty set instead of a remove.

--- a/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
+++ b/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
@@ -208,9 +208,8 @@ public abstract class FSDirectory extends BaseDirectory {
     ensureOpen();
     maybeDeletePendingFiles();
     // If this file was pending delete, we are now bringing it back to life:
-    if (pendingDeletes.remove(name)) {
-      privateDeleteFile(name, true); // try again to delete it - this is the best effort
-      pendingDeletes.remove(name); // watch out - if the delete fails it put
+    if (pendingDeletes.contains(name)) {
+      revivePendingDelete(name);
     }
     return new FSIndexOutput(name);
   }
@@ -258,9 +257,8 @@ public abstract class FSDirectory extends BaseDirectory {
           "file \"" + source + "\" is pending delete and cannot be moved");
     }
     maybeDeletePendingFiles();
-    if (pendingDeletes.remove(dest)) {
-      privateDeleteFile(dest, true); // try again to delete it - this is the best effort
-      pendingDeletes.remove(dest); // watch out if the delete fails, it's back in here
+    if (pendingDeletes.contains(dest)) {
+      revivePendingDelete(dest);
     }
     Files.move(directory.resolve(source), directory.resolve(dest), StandardCopyOption.ATOMIC_MOVE);
   }
@@ -322,6 +320,18 @@ public abstract class FSDirectory extends BaseDirectory {
     }
   }
 
+  /**
+   * Takes {@code name} out of the pending-delete set, retrying the delete of the old file first.
+   * Synchronized like {@link #deletePendingFiles()}, so that a concurrent retry loop that already
+   * copied the pending set cannot delete the new file created under this name afterwards.
+   */
+  private synchronized void revivePendingDelete(String name) throws IOException {
+    if (pendingDeletes.remove(name)) {
+      privateDeleteFile(name, true); // try again to delete it - this is the best effort
+      pendingDeletes.remove(name); // watch out - if the delete fails it put
+    }
+  }
+
   private void maybeDeletePendingFiles() throws IOException {
     if (pendingDeletes.isEmpty() == false) {
       // This is a silly heuristic to try to avoid O(N^2), where N = number of files pending

Trade-offs

A createOutput or rename of a pending name now waits for another thread's running retry loop (one delete attempt per pending file). An application that synchronizes on the Directory instance, which Directory.java:43-45 warns against since #11619 (LUCENE-10583, a deadlock against deletePendingFiles on Windows), can now also block such a revive; the existing retry path already has that exposure.

What the checks validated

• Scratch worktree of main 645d4fe14a20 (removed afterwards), seed DEADBEEF: all of :lucene:core:test with A and the repro added, assertions on: 8,898 tests in 749 classes, 322 skipped, 0 failures.
• The closest suites with A, -Ptests.asserts=false: org.apache.lucene.store.* (including the repro), TestIndexWriter, TestNRTReaderCleanup, TestIndexFileDeleter: 908 tests, 18 skipped, 0 failures.
• Repro harness with A shadowed in: passes with assertions on and off and 10 of 10 iterations; the hook-free stress test loses 0 of 2,000 re-created files with no spurious NoSuchFileException.

The same helper, called unconditionally: revivePendingDelete(name); in createOutput and revivePendingDelete(dest); in rename.

-    if (pendingDeletes.remove(name)) {
-      privateDeleteFile(name, true); // try again to delete it - this is the best effort
-      pendingDeletes.remove(name); // watch out - if the delete fails it put
-    }
+    revivePendingDelete(name);

Trade-offs

Every createOutput and rename now enters the monitor, even when nothing is pending, and so waits for any concurrent retry or close(). Simpler to read than A; slightly more contention.

What the checks validated

• Scratch worktree of main 645d4fe14a20 (removed afterwards), seed DEADBEEF: stock, the repro fails through gradle; with B it passes together with the closest suites (908 tests, 18 skipped, 0 failures) with assertions on and off, and all of :lucene:core:test (8,898 tests, 322 skipped, 0 failures, assertions on).
• Earlier, by the hunt: the stress test lost 0 of 2,000 files; the history probe's createOutput, rename and control scenarios all keep the file; the hunt's latch test passes.

if (pendingDeletes.contains(name)) privateDeleteFile(name, true); inside deletePendingFiles.

Trade-offs

A check-then-act: a revive between the check and Files.delete still loses the file, so it only narrows the window. A diagnostic copy of it made the independent reproducer's latch tests pass, which shows the stale copy is the cause, not that the race is gone.

What the checks validated

Only as a diagnostic, by the independent reproducer.

Document that a name whose delete is pending must not be re-used while other threads use the directory.

Trade-offs

Contradicts the explicit revive code and LUCENE-8318's precedent; not recommended.

• Method (semi-formal): TLA+ model checking. In a hunt unit on locks and pending deletes, the finder modelled FSDirectory's pending-delete protocol in TLA+: a re-create thread, the retry loop and an operating system that refuses a delete and later allows it.
• Signal. The hypothesis came from reading the code while writing the model. TLC then violated the invariant NewFileSurvives ("a successfully created, never-deleted file stays visible with its content") in a 10-state trace on the stock semantics; with the synchronization fix it explored 122 distinct states without error. Two vacuity probes confirmed that the create and the retry's delete are reachable.
• Reduction. The finder wrote a public-API JUnit with latches on the stock jars (fails 20 of 20); it was later minimized to the one test above.
• The finder labelled it formal; the registry records semi-formal, because the first signal was a code-reading hypothesis that the model checker then witnessed.