From 1617bf1485c644b1c3393f865c9043cdd68bb6d5 Mon Sep 17 00:00:00 2001
From: Jake Landis <jake.landis@elastic.co>
Date: Thu, 31 Aug 2023 19:21:23 -0500
Subject: [PATCH] add optimistic concurrency

---
 .../security/support/RotatableSecret.java     | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/RotatableSecret.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/RotatableSecret.java
index 0e4fc25519771..c9c7ebbda9392 100644
--- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/RotatableSecret.java
+++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/RotatableSecret.java
@@ -18,11 +18,11 @@
 /**
  * Helper class to provide a secret that can be rotated. Once rotated the prior secret is available for a configured amount of time before
  * it is invalidated. This allows for secrete rotation without temporary failures or the need to tightly orchestrate multiple parties.
- * This class is threadsafe, however it is also assumes that matching secrets are frequent and rotation is a rare.
+ * This class is threadsafe, however it is also assumes that matching secrets are frequent but rotation is a rare.
  */
 public class RotatableSecret {
     private Secrets secrets;
-    private final StampedLock stampedLock = new StampedLock(); //read/write lock that allow upgrading from read to write
+    private final StampedLock stampedLock = new StampedLock();
 
     /**
      * @param secret The secret to rotate. {@code null} if the secret is not configured.
@@ -80,20 +80,31 @@ public Secrets getSecrets() {
     }
 
     private void checkExpired() {
-        long stamp = stampedLock.readLock();
+        boolean needToUnlock = false;
+        long stamp = stampedLock.tryOptimisticRead();
+        boolean expired = secrets.prior != null && secrets.priorValidTill.isBefore(Instant.now()); // optimistic read
+        if (stampedLock.validate(stamp) == false) {
+            // optimism failed...potentially block to obtain the read lock and try the read again
+            stamp = stampedLock.readLock();
+            needToUnlock = true;
+            expired = secrets.prior != null && secrets.priorValidTill.isBefore(Instant.now()); // locked read
+        }
         try {
-            if (secrets.prior != null && secrets.priorValidTill.isBefore(Instant.now())) {
-                stamp = stampedLock.tryConvertToWriteLock(stamp);
+            if (expired) {
+                stamp = stampedLock.tryConvertToWriteLock(stamp);// upgrade the read lock
                 if (stamp == 0) {
                     // block until we can acquire the write lock
                     stamp = stampedLock.writeLock();
                 }
+                needToUnlock = true;
                 SecureString prior = secrets.prior;
                 secrets = new Secrets(secrets.current, null, Instant.EPOCH);
-                prior.close(); //zero out the memory
+                prior.close(); // zero out the memory
             }
         } finally {
-            stampedLock.unlock(stamp);
+            if (needToUnlock) { // only unlock if we acquired a read or write lock
+                stampedLock.unlock(stamp);
+            }
         }
     }