From e0243e31c59a4b458e1719b52ed45d53333f9bb8 Mon Sep 17 00:00:00 2001
From: Marko Topolnik <marko.topolnik@gmail.com>
Date: Fri, 11 Sep 2026 11:01:28 +0200
Subject: [PATCH] Make token-store rename retry test deterministic

Add a private per-call retry hook so the test restores directory write
permissions after a real denied rename on the calling thread. Assert
that exactly one retry completes the atomic replacement and consumes
the temporary file.

Remove the helper thread and its scheduling dependency on the short
retry budget. Keep production retry limits and interrupt handling
unchanged, and update the test's platform-scope comments.

Validation: all 90 FileTokenStoreTest tests pass through Maven on
macOS arm64 with Java 8 and Java 25.
---
 .../client/cutlass/auth/FileTokenStore.java   |  8 ++++
 .../test/cutlass/auth/FileTokenStoreTest.java | 40 ++++++-------------
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java b/core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java
index 489ef213..cde281bb 100644
--- a/core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java
+++ b/core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java
@@ -987,6 +987,10 @@ private static void releaseProcessLock(String identity) {
 
 
     private static void replaceTarget(Path tmp, Path target) throws IOException {
+        replaceTarget(tmp, target, null);
+    }
+
+    private static void replaceTarget(Path tmp, Path target, Runnable beforeRetryForTesting) throws IOException {
         // atomically rename tmp over target. On Windows a concurrent reader in any process holding target open
         // can make the rename fail transiently with AccessDeniedException (a sharing violation); retry a few
         // times on a short backoff before giving up, so a routine read/write overlap does not needlessly degrade
@@ -1010,6 +1014,10 @@ private static void replaceTarget(Path tmp, Path target) throws IOException {
                     Thread.currentThread().interrupt();
                     break;
                 }
+                // Tests clear a real permission denial before the next move attempt.
+                if (beforeRetryForTesting != null) {
+                    beforeRetryForTesting.run();
+                }
             }
             try {
                 Files.move(tmp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
diff --git a/core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java b/core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java
index 264c11b5..7d66ce3f 100644
--- a/core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java
+++ b/core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java
@@ -73,8 +73,7 @@
 /**
  * Coverage for {@link FileTokenStore}.
  * <p>
- * PLATFORM SCOPE. CI runs Linux only, so the store's Windows-motivated arms are covered here to the extent a
- * POSIX host can reach them, and no further:
+ * PLATFORM SCOPE. POSIX hosts cover the store's Windows-motivated arms only to the extent described below:
  * <ul>
  *     <li>the {@code AccessDeniedException} retry in {@code replaceTarget} - the sharing violation a Windows
  *     reader holding the target open produces - IS exercised, by denying the rename with directory
@@ -765,47 +764,34 @@ public void testReplaceTargetRetriesADeniedRenameThenSucceeds() throws Exception
         Assume.assumeFalse("a root process bypasses the directory permissions this denial relies on",
                 "root".equals(System.getProperty("user.name")));
         assertMemoryLeak(() -> {
-            // replaceTarget retries a denied rename because on WINDOWS a concurrent reader holding the target
-            // open makes the atomic replace fail transiently with AccessDeniedException. CI is Linux-only, so
-            // the denial is produced the one way a POSIX host can: rename(2) needs write permission on the
-            // containing directory, so taking it away denies the move exactly as the sharing violation does,
-            // and restoring it mid-retry stands in for the Windows reader closing its handle.
+            // A POSIX directory permission denial produces the same AccessDeniedException as a Windows
+            // sharing violation. Clear it on the calling thread after the first denied move.
             Path dir = storeDir();
             createStoreDir(dir);
             Path tmp = Files.write(dir.resolve("payload.tmp"), "NEW".getBytes(StandardCharsets.UTF_8));
             Path target = Files.write(dir.resolve("payload.json"), "OLD".getBytes(StandardCharsets.UTF_8));
 
-            Method replaceTarget = FileTokenStore.class.getDeclaredMethod("replaceTarget", Path.class, Path.class);
+            Method replaceTarget = FileTokenStore.class.getDeclaredMethod(
+                    "replaceTarget", Path.class, Path.class, Runnable.class);
             replaceTarget.setAccessible(true);
-            Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("r-x------"));
-            // Prove the denial is real on THIS host before the test rests on it. Without this the whole test
-            // passes vacuously wherever the mode bits do not bite - the first attempt inside replaceTarget
-            // simply succeeds and no retry is ever exercised.
-            try {
-                Files.move(tmp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
-                Assert.fail("the rename must be denied while the store directory is not writable");
-            } catch (AccessDeniedException expected) {
-                // exactly what a Windows sharing violation produces, and what the retry loop is written for
-            }
-            Thread reopener = new Thread(() -> {
-                // after the first backoff (20ms) but well inside the 5-attempt budget
-                Os.sleep(30);
+            AtomicInteger retries = new AtomicInteger();
+            Runnable clearDenial = () -> {
+                retries.incrementAndGet();
                 try {
                     Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("rwx------"));
                 } catch (IOException e) {
                     throw new AssertionError("could not restore the directory permissions", e);
                 }
-            }, "denial-clearer");
-            reopener.setDaemon(true);
-            reopener.start();
+            };
+            Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("r-x------"));
             try {
-                replaceTarget.invoke(null, tmp, target);
+                replaceTarget.invoke(null, tmp, target, clearDenial);
             } finally {
-                reopener.join(10_000);
-                // whatever happened above, leave the tree deletable
+                // Leave the tree deletable even if the move or the retry hook fails.
                 Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("rwx------"));
             }
 
+            Assert.assertEquals("the first move must be denied and the first retry must succeed", 1, retries.get());
             Assert.assertEquals("the retry must complete the replace once the denial clears",
                     "NEW", new String(Files.readAllBytes(target), StandardCharsets.UTF_8));
             Assert.assertFalse("an atomic move consumes the temp file", Files.exists(tmp));