Merge pull request 'fix: a reveal and the access-log writer no longer wait for each other' (#45) from fix/reveal-access-log-deadlock into develop

A reveal, and the start, end and expiry of a break-glass session, hold the
runtime's monitor from their first check to their last write. Asking for a
classification (SecuredDataRuntime.classificationRows, and
activeRetentionRules) took the same monitor, and every thread asks: core
asks for every field it decides on and for every filter criterion of every
query, including the queries made inside this QBit's own writes.

So a thread that asked while holding another lock of this QBit could not be
answered during a reveal, and if the reveal needed that lock next, both
waited for ever. The access-log writer is such a thread. It holds the
anomaly counters while it inserts a row; the insert counts earlier rows and
writes an audit, both queries with criteria. A reveal inserts its own
access-log row.

    reveal:  runtime's monitor, then the anomaly counters
    writer:  the anomaly counters, then the runtime's monitor

Seen in a host on PostgreSQL: a person read a record with a masked field and
revealed it at once; the writer's tick drained the read's log rows at the
same moment. The request and the writer deadlocked (thread dump: the request
in SystemWrites.insertOne under RevealService.reveal, the writer in
SecuredDataRuntime.classificationRows under AuditAction.fetchIdFromName
under its own insert), and every later request blocked in
classificationRows. The same order existed between a reveal and the
resolver's monitor, and the key source's.

- The rule rows have a lock of their own (ruleLock), held only while they
  are answered or read. A read is a query made as the system and takes none
  of the QBit's other locks, so nothing that holds ruleLock waits for one.
- The runtime's monitor is left to what it was for: one reveal or
  break-glass change at a time.

Tests (SecuredDataLockOrderTest, on H2 in PostgreSQL mode with the audit
tables): a thread is parked inside a reveal, and inside the start of a
break-glass session, holding the runtime's monitor; the access-log writer
must drain its queue and another thread must be told a classification and
the active retention rules meanwhile, and then the parked operation must
finish. Both tests fail without the fix (neither thread finishes).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes are documented here, following [Keep a Changelog](https://ke
 
 ## [Unreleased]
 
+### Fixed: a reveal and the access-log writer waiting for each other
+- A reveal, or the start, end or expiry of a break-glass session, no longer deadlocks with a thread that asks for a classification while it holds another lock of this QBit. The access-log writer is such a thread: it holds the anomaly counters while it inserts a row, and that insert asks for classifications (it counts earlier rows and writes an audit, both queries with criteria). A reveal holds the runtime's monitor and then inserts its own access-log row. When both happened at once, neither finished, and neither did any later request, because every request asks for a classification. A person who opens a record with a masked field and reveals it straight away does both at once.
+- The rule rows a node keeps (classifications, active retention rules) have a lock of their own, held only while they are answered or read. Asking for a classification no longer takes the monitor that reveal and break-glass hold for their whole operation.
+
+### Fixed: rule reads that ask for their own rows
+- Reading the rule rows a node keeps (classifications, active retention rules, policies) no longer recurses when code that runs inside that read asks for the same rows. Another QBit's virtual field value provider, or a post-query customizer, on a secured-data table that ran a query with filter criteria did exactly that: with one classification row present, every request that needed a classification ended in a `StackOverflowError`.
+- The QBit's system reads of its own tables (`SystemWrites.query`, the audit key lookup) name the stored fields of the table, so core runs no virtual field value provider inside them.
+- A question asked from inside a rule read is answered from the rows held when that read began (none before the first read) and starts no other read. Only the reading thread, inside the QBit's system read, can be given that answer; every request gets the rows the read returns. The same holds for a transaction's read of its own retention rule edits.
+- A rule read that fails is tried again by the next question instead of failing or recursing from then on.
+
 ### Fixed: access-log lifecycle
 - The access-log worker and JVM shutdown hook hold the logger weakly, so an instance the host drops (tests, reloads) can be collected; its worker stops at the next tick.
 - `withRegisterShutdownHook(false)` for hosts that call `SecuredDataRuntime.close()` themselves; `SecuredDataAccessLogger.isWorkerRunning()`.

src/main/java/com/kingsrook/qbits/secureddata/RuleRows.java

@@ -0,0 +1,103 @@
+/*
+ * QQQ - Low-code Application Framework for Engineers.
+ * Copyright (C) 2021-2025.  Kingsrook, LLC
+ * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States
+ * contact@kingsrook.com
+ * https://github.com/Kingsrook/
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.kingsrook.qbits.secureddata;
+
+
+import java.util.List;
+import java.util.function.Supplier;
+import com.kingsrook.qqq.backend.core.model.data.QRecord;
+
+
+/*******************************************************************************
+ ** One kind of rule rows a node keeps in memory (classifications, policies or
+ ** active retention rules): read once per version, never used unread past
+ ** SecuredDataRuntime.MAX_RULE_AGE, and safe against a read that asks for the
+ ** same rows again.
+ **
+ ** Not synchronized: every call must be made holding the owner's lock (the
+ ** runtime's rule lock for classifications and retention rules, the resolver's
+ ** monitor for policies). That lock is what makes the re-entry answer below
+ ** private to one thread: while a read is running, only the thread running it
+ ** can get in here; every other thread waits for the lock and is given the rows
+ ** that read returns.
+ **
+ ** Re-entry: reading rule rows is itself a query, and anything that runs inside
+ ** it (a table customizer, a record-security lookup, a value provider) may ask a
+ ** question that needs the very rows being read. Such a nested question is
+ ** answered with the rows this node held when the read began - the answer every
+ ** request was given until now, and no rows before the first read - and never
+ ** starts another read. The request that started the read gets the new rows.
+ **
+ ** Failure: a read that throws leaves the previous rows and version in place
+ ** and is not remembered, so the next question reads again. The previous rows
+ ** are never handed to a request in place of rows that could not be read.
+ *******************************************************************************/
+final class RuleRows
+{
+   private List<QRecord> rows = List.of();
+   private long          version;
+   private long          loadedAt;
+   private boolean       loaded;
+   private boolean       reading;
+
+
+
+   /*******************************************************************************
+    ** The rows of this version, reading them when the rows held are of another
+    ** version, too old, or were never read.
+    **
+    ** @param currentVersion the owner's invalidation clock, read by the caller
+    **                       before this call: a change during the read leaves a
+    **                       version that no longer matches, so the next question
+    **                       reads again
+    ** @param read           the query; whatever it throws is thrown to the caller
+    *******************************************************************************/
+   List<QRecord> get(long currentVersion, Supplier<List<QRecord>> read)
+   {
+      if(loaded && version == currentVersion && !SecuredDataRuntime.tooOld(loadedAt))
+      {
+         return (rows);
+      }
+      if(reading)
+      {
+         ///////////////////////////////////////////////////////////////////////////
+         // the read that is running on this thread asked for its own rows: give //
+         // it the rows held when it began, and do not start another read        //
+         ///////////////////////////////////////////////////////////////////////////
+         return (rows);
+      }
+      reading = true;
+      try
+      {
+         long          startedAt = System.nanoTime();
+         List<QRecord> fresh     = List.copyOf(read.get());
+         rows = fresh;
+         version = currentVersion;
+         loadedAt = startedAt;
+         loaded = true;
+      }
+      finally
+      {
+         reading = false;
+      }
+      return (rows);
+   }
+}

src/main/java/com/kingsrook/qbits/secureddata/SecuredDataKeySource.java

@@ -124,7 +124,7 @@ public final class SecuredDataKeySource implements ClassifiedValueKeySource, Aut
       try
       {
          List<QRecord> rows = SystemWrites.call(() -> new QueryAction().execute(new QueryInput(SecuredDataTables.AUDIT_KEY)
-            .withFilter(recordField(table, field, pk)).withShouldOmitHiddenFields(false)
+            .withFilter(recordField(table, field, pk)).withShouldOmitHiddenFields(false).withFieldNamesToInclude(SystemWrites.storedFields(SecuredDataTables.AUDIT_KEY))
             .withTransaction(QContext.getQBackendTransaction())).getRecords());
          if(rows.isEmpty())
          {

src/main/java/com/kingsrook/qbits/secureddata/SecuredDataResolver.java

@@ -74,8 +74,8 @@ public class SecuredDataResolver implements FieldAccessResolverInterface
    {
    }
 
-   private long cachedVersion;
-   private long policiesLoadedAt;
+   /** Used only inside methods synchronized on this resolver. */
+   private final RuleRows policyRows = new RuleRows();
    private List<QRecord> policies = List.of();
    private final Map<String, UserState> users = new HashMap<>();
 
@@ -292,18 +292,19 @@ public class SecuredDataResolver implements FieldAccessResolverInterface
 
    /*******************************************************************************
     ** Refresh policy rows once per version and matched state per relevant permissions.
+    ** A read that fails throws and is tried again by the next call. A question
+    ** asked from inside the read itself (see RuleRows) is answered from the
+    ** policies held when the read began; whatever it matched for its user is
+    ** dropped with every other user's state when the read returns.
     *******************************************************************************/
    private synchronized UserState state()
    {
-      long version = runtime.policyVersion();
-      if(cachedVersion != version || SecuredDataRuntime.tooOld(policiesLoadedAt))
+      policies = policyRows.get(runtime.policyVersion(), () ->
       {
-         long loadedAt = System.nanoTime();
-         policies = List.copyOf(SystemWrites.query(SecuredDataTables.POLICY, new QQueryFilter()));
+         List<QRecord> rows = SystemWrites.query(SecuredDataTables.POLICY, new QQueryFilter());
          users.clear();
-         cachedVersion = version;
-         policiesLoadedAt = loadedAt;
-      }
+         return (rows);
+      });
       QSession session = QContext.getQSession();
       TreeSet<String> held = new TreeSet<>();
       for(QRecord policy : policies)

src/main/java/com/kingsrook/qbits/secureddata/SecuredDataRuntime.java

@@ -78,12 +78,26 @@ public class SecuredDataRuntime implements QSupplementalInstanceMetaData, AutoCl
    private final transient SecuredDataKeySource keys = new SecuredDataKeySource();
    private final transient SecuredDataAccessLogger logger;
    private final transient SecuredDataAnomalies anomalies;
-   private transient long cachedClassificationVersion;
-   private transient List<com.kingsrook.qqq.backend.core.model.data.QRecord> classificationRows = List.of();
-   private transient long classificationRowsLoadedAt;
-   private transient long cachedRetentionVersion;
-   private transient long retentionRulesLoadedAt;
-   private transient List<QRecord> activeRetentionRules = List.of();
+   /////////////////////////////////////////////////////////////////////////////////
+   // both are used only while holding ruleLock: one lock for the two, because    //
+   // reading either may ask for the other.                                       //
+   //                                                                             //
+   // It is a lock of its own, never this runtime's monitor. A reveal and the     //
+   // start, end and expiry of a break-glass session hold the runtime's monitor   //
+   // for their whole operation, and every thread asks for a classification all   //
+   // the time (core asks for every field it decides on and for every criterion   //
+   // of every query), including threads that hold another lock of this QBit: the //
+   // access-log writer inside the anomaly counters, the resolver, the key        //
+   // source. When the question needed the runtime's monitor, such a thread and   //
+   // a reveal that needed its lock next waited for each other for ever.          //
+   //                                                                             //
+   // ruleLock is held only while rule rows are answered or read. A read is a     //
+   // query made as the system: it takes none of the QBit's other locks, so no    //
+   // thread holding ruleLock ever waits for one of them.                         //
+   /////////////////////////////////////////////////////////////////////////////////
+   private final transient Object   ruleLock             = new Object();
+   private final transient RuleRows classificationRows   = new RuleRows();
+   private final transient RuleRows activeRetentionRules = new RuleRows();
 
 
 
@@ -228,20 +242,17 @@ public class SecuredDataRuntime implements QSupplementalInstanceMetaData, AutoCl
    /*******************************************************************************
     ** Every active retention rule, read once per version and at most MAX_RULE_AGE
     ** old. A read that fails throws and leaves nothing in place of the rules:
-    ** callers never see "no rules" because the rules could not be read.
+    ** callers never see "no rules" because the rules could not be read, and the
+    ** next call reads again. A question asked from inside the read itself (see
+    ** RuleRows) is answered with the rules held when the read began.
     *******************************************************************************/
-   public synchronized List<QRecord> activeRetentionRules()
+   public List<QRecord> activeRetentionRules()
    {
-      long version = retentionVersion();
-      if(cachedRetentionVersion != version || tooOld(retentionRulesLoadedAt))
+      synchronized(ruleLock)
       {
-         long loadedAt = System.nanoTime();
-         activeRetentionRules = List.copyOf(SystemWrites.query(SecuredDataTables.RETENTION_RULE, new QQueryFilter()
-            .withCriteria(new QFilterCriteria("is_active", QCriteriaOperator.EQUALS, true))));
-         cachedRetentionVersion = version;
-         retentionRulesLoadedAt = loadedAt;
+         return (activeRetentionRules.get(retentionVersion(), () -> SystemWrites.query(SecuredDataTables.RETENTION_RULE, new QQueryFilter()
+            .withCriteria(new QFilterCriteria("is_active", QCriteriaOperator.EQUALS, true)))));
       }
-      return (activeRetentionRules);
    }
 
 
@@ -290,20 +301,17 @@ public class SecuredDataRuntime implements QSupplementalInstanceMetaData, AutoCl
 
 
    /*******************************************************************************
-    ** Read classification rows once per version, always as the system.
+    ** Read classification rows once per version, always as the system. A read
+    ** that fails throws and is tried again by the next call. A question asked from
+    ** inside the read itself (see RuleRows) is answered with the rows held when
+    ** the read began, and never starts another read.
     *******************************************************************************/
-   synchronized List<com.kingsrook.qqq.backend.core.model.data.QRecord> classificationRows()
+   List<QRecord> classificationRows()
    {
-      long version = classificationVersion();
-      if(cachedClassificationVersion != version || tooOld(classificationRowsLoadedAt))
+      synchronized(ruleLock)
       {
-         long loadedAt = System.nanoTime();
-         classificationRows = List.copyOf(com.kingsrook.qbits.secureddata.support.SystemWrites.query(
-            com.kingsrook.qbits.secureddata.metadata.SecuredDataTables.CLASSIFICATION, new com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter()));
-         cachedClassificationVersion = version;
-         classificationRowsLoadedAt = loadedAt;
+         return (classificationRows.get(classificationVersion(), () -> SystemWrites.query(SecuredDataTables.CLASSIFICATION, new QQueryFilter())));
       }
-      return (classificationRows);
    }
 
 

src/main/java/com/kingsrook/qbits/secureddata/retention/RetentionRules.java

@@ -65,6 +65,9 @@ public final class RetentionRules
    ///////////////////////////////////////////////////////////////////////////////
    private static final ThreadLocal<Boolean> EDITED_IN_TRANSACTION = ThreadLocal.withInitial(() -> false);
 
+   /** Set while this thread reads its own edits, so that a question from inside that read does not read them again. */
+   private static final ThreadLocal<Boolean> READING_OWN_EDITS = ThreadLocal.withInitial(() -> false);
+
 
 
    /*******************************************************************************
@@ -74,21 +77,36 @@ public final class RetentionRules
     ** change, and at most MAX_RULE_AGE old) instead of once per call. A committed
     ** change is seen at once on the node that made it and on every node the
     ** cluster event reaches; a transaction that edits rules reads its own edits.
-    ** A failed read throws: it is never "no rules".
+    ** A failed read throws: it is never "no rules". A question asked from inside
+    ** a read of the rules is answered from the rules held before that read (see
+    ** RuleRows) and never starts another.
     *******************************************************************************/
    public static List<QRecord> active(String table, String field)
    {
       SecuredDataRuntime runtime = SecuredDataRuntime.current();
       List<QRecord> rules;
-      if(runtime == null || readsOwnEdits())
+      if(runtime != null && (!readsOwnEdits() || READING_OWN_EDITS.get()))
       {
-         rules = SystemWrites.query(SecuredDataTables.RETENTION_RULE, new QQueryFilter()
-            .withCriteria(new QFilterCriteria("is_active", QCriteriaOperator.EQUALS, true))
-            .withCriteria(new QFilterCriteria("table_name", QCriteriaOperator.EQUALS, table)));
+         //////////////////////////////////////////////////////////////////////////
+         // also the answer to a question asked from inside this thread's read  //
+         // of its own edits (code that runs inside that query): the rules      //
+         // every other thread shares, and never another read of the edits      //
+         //////////////////////////////////////////////////////////////////////////
+         rules = runtime.activeRetentionRules();
       }
       else
       {
-         rules = runtime.activeRetentionRules();
+         READING_OWN_EDITS.set(true);
+         try
+         {
+            rules = SystemWrites.query(SecuredDataTables.RETENTION_RULE, new QQueryFilter()
+               .withCriteria(new QFilterCriteria("is_active", QCriteriaOperator.EQUALS, true))
+               .withCriteria(new QFilterCriteria("table_name", QCriteriaOperator.EQUALS, table)));
+         }
+         finally
+         {
+            READING_OWN_EDITS.remove();
+         }
       }
       return (rules.stream().filter(rule -> table != null && table.equals(rule.getValueString("table_name"))).filter(rule -> fields(rule).contains(field)).toList());
    }

src/main/java/com/kingsrook/qbits/secureddata/support/SystemWrites.java

@@ -21,7 +21,9 @@
 package com.kingsrook.qbits.secureddata.support;
 
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import com.kingsrook.qqq.backend.core.actions.QBackendTransaction;
 import com.kingsrook.qqq.backend.core.actions.permissions.CorePermissionEnforcer;
 import com.kingsrook.qqq.backend.core.actions.tables.InsertAction;
@@ -32,6 +34,7 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput;
 import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter;
 import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput;
 import com.kingsrook.qqq.backend.core.model.data.QRecord;
+import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
 import com.kingsrook.qqq.backend.core.model.session.QSession;
 import com.kingsrook.qqq.backend.core.model.session.QSystemUserSession;
 
@@ -100,13 +103,15 @@ public final class SystemWrites
 
 
    /*******************************************************************************
-    ** Query without user locks or field access recursion.
+    ** Query without user locks or field access recursion, for the stored fields of
+    ** the table only (see storedFields).
     *******************************************************************************/
    public static List<QRecord> query(String table, QQueryFilter filter)
    {
       try
       {
-         return (call(() -> new QueryAction().execute(new QueryInput(table).withFilter(filter).withTransaction(QContext.getQBackendTransaction())).getRecords()));
+         return (call(() -> new QueryAction().execute(new QueryInput(table).withFilter(filter).withFieldNamesToInclude(storedFields(table))
+            .withTransaction(QContext.getQBackendTransaction())).getRecords()));
       }
       catch(QException e)
       {
@@ -116,6 +121,22 @@ public final class SystemWrites
 
 
 
+   /*******************************************************************************
+    ** The names of the stored fields of a table, as the projection of a system
+    ** read. A query that names its fields is not given virtual fields, so core
+    ** runs none of the value providers another QBit or the host may have added to
+    ** a secured-data table: this QBit reads its own columns and nothing else, and
+    ** a provider cannot be made to run inside a rule read. Null (every field, and
+    ** the error core reports) for a table the instance does not have.
+    *******************************************************************************/
+   public static Set<String> storedFields(String tableName)
+   {
+      QTableMetaData table = QContext.getQInstance().getTable(tableName);
+      return (table == null ? null : new HashSet<>(table.getFields().keySet()));
+   }
+
+
+
    /*******************************************************************************
     ** Insert and reject backend record errors.
     *******************************************************************************/

src/test/java/com/kingsrook/qbits/secureddata/SecuredDataLockOrderTest.java

@@ -0,0 +1,271 @@
+/*
+ * QQQ - Low-code Application Framework for Engineers.
+ * Copyright (C) 2021-2025.  Kingsrook, LLC
+ * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States
+ * contact@kingsrook.com
+ * https://github.com/Kingsrook/
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.kingsrook.qbits.secureddata;
+
+
+import java.io.Serializable;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import com.kingsrook.qbits.secureddata.breakglass.BreakGlassService;
+import com.kingsrook.qbits.secureddata.metadata.SecuredDataTables;
+import com.kingsrook.qbits.secureddata.reveal.RevealResult;
+import com.kingsrook.qbits.secureddata.reveal.RevealService;
+import com.kingsrook.qqq.backend.core.actions.ai.DataClassification;
+import com.kingsrook.qqq.backend.core.actions.permissions.fieldaccess.FieldAccessEvent;
+import com.kingsrook.qqq.backend.core.actions.permissions.fieldaccess.FieldAccessEventType;
+import com.kingsrook.qqq.backend.core.actions.permissions.fieldaccess.FieldAccessOutcome;
+import com.kingsrook.qqq.backend.core.context.QContext;
+import com.kingsrook.qqq.backend.core.model.data.QRecord;
+import com.kingsrook.qqq.backend.core.model.metadata.QInstance;
+import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
+import com.kingsrook.qqq.backend.core.model.session.QSession;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.CLOCK;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.insert;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.logs;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.policyRow;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+
+
+/*******************************************************************************
+ ** A reveal, and the start, end and expiry of a break-glass session, hold the
+ ** runtime's monitor from their first check to their last write, so that two of
+ ** them cannot both pass a check (the daily allowance, one active session).
+ ** Every other thread asks this QBit for a classification all the time: core
+ ** asks for every field it decides on and for every filter criterion of every
+ ** query, including the queries made inside this QBit's own writes.
+ **
+ ** When that question needed the same monitor, a thread that asked it while
+ ** holding another of this QBit's locks could never be answered during a
+ ** reveal that was about to need that lock: the access-log writer holds the
+ ** anomaly counters' monitor while it inserts a row (the insert counts earlier
+ ** rows and writes an audit, both queries with criteria), and a reveal inserts
+ ** its own access-log row. Reveal: runtime, then counters. Writer: counters,
+ ** then runtime. Both waited for ever, and so did every later request, since
+ ** every request asks for a classification.
+ **
+ ** The rule rows now have a lock of their own that is held only while they are
+ ** read. These tests run on a relational database (H2 in PostgreSQL mode, with
+ ** the audit tables), where an insert really writes its audit. Each parks one
+ ** thread inside the operation, holding the runtime's monitor, and requires
+ ** the access-log writer and a classification question on other threads to
+ ** finish meanwhile; then the parked operation finishes too. Without the fix
+ ** the writer does not finish and the test fails after WAIT_SECONDS.
+ *******************************************************************************/
+@Timeout(180)
+class SecuredDataLockOrderTest
+{
+   private static final long WAIT_SECONDS = 20;
+
+   private final ExecutorService threads = Executors.newCachedThreadPool(work ->
+   {
+      Thread thread = new Thread(work, "lock-order-test");
+      thread.setDaemon(true);
+      return (thread);
+   });
+
+   private QInstance          instance;
+   private SecuredDataRuntime runtime;
+   private QSession           person;
+   private QTableMetaData     reading;
+   private Serializable       pk;
+
+   private final CountDownLatch          parked  = new CountDownLatch(1);
+   private final CountDownLatch          resume  = new CountDownLatch(1);
+   private final AtomicReference<Thread> working = new AtomicReference<>();
+   private final AtomicBoolean           heldTheRuntimesMonitor = new AtomicBoolean();
+
+
+
+   /*******************************************************************************
+    ** Let a parked thread go, then close; a runtime whose threads are stuck was
+    ** already taken out of the list (see abandon).
+    *******************************************************************************/
+   @AfterEach
+   void cleanup()
+   {
+      resume.countDown();
+      threads.shutdownNow();
+      RetentionTestSupport.close();
+   }
+
+
+
+   /*******************************************************************************
+    ** A reveal in progress holds up neither the access-log writer nor a question.
+    *******************************************************************************/
+   @Test
+   void theAccessLogWriterAndAClassificationQuestionFinishWhileARevealIsInProgress() throws Exception
+   {
+      setup();
+      Future<RevealResult> reveal = parkInside(() -> new RevealService().reveal("reading", pk, "notes", null, "Synthetic reveal; lock order test."));
+
+      othersFinishMeanwhile();
+
+      resume.countDown();
+      assertThat(reveal.get(WAIT_SECONDS, TimeUnit.SECONDS).value()).isEqualTo("test-notes");
+      assertThat(logs()).extracting(row -> row.getValueString("event_type")).contains("EXACT_MATCH_FILTER", "REVEAL");
+      assertThat(runtime.resolver().isRevealActive("reading", "notes", pk)).isTrue();
+   }
+
+
+
+   /*******************************************************************************
+    ** The same for the start of a break-glass session, which holds the same monitor.
+    *******************************************************************************/
+   @Test
+   void theAccessLogWriterAndAClassificationQuestionFinishWhileBreakGlassStarts() throws Exception
+   {
+      setup();
+      person.getPermissions().add(BreakGlassService.START + ".hasAccess");
+      Future<QRecord> started = parkInside(() -> new BreakGlassService().start("Synthetic break-glass session; lock order test.", 5, "reading", DataClassification.PHI));
+
+      othersFinishMeanwhile();
+
+      resume.countDown();
+      assertThat(started.get(WAIT_SECONDS, TimeUnit.SECONDS).getValueString("status")).isEqualTo("ACTIVE");
+      assertThat(logs()).extracting(row -> row.getValueString("event_type")).contains("EXACT_MATCH_FILTER", "BREAK_GLASS_START");
+   }
+
+
+
+   /*******************************************************************************
+    ** A relational instance with audit tables; a PII field one policy makes
+    ** revealable to the person; one row; and one search of that field waiting in
+    ** the access-log queue, as it waits after a person filters a list by it.
+    *******************************************************************************/
+   private void setup() throws Exception
+   {
+      instance = RetentionTestSupport.setup();
+      runtime = SecuredDataRuntime.current();
+      person = QContext.getQSession();
+      reading = instance.getTable("reading");
+      insert(SecuredDataTables.POLICY, policyRow("secured_data.admin", "PII", "REVEALABLE"));
+      pk = RetentionTestSupport.reading(120).getValue("id");
+
+      runtime.resolver().onAccess(new FieldAccessEvent().withType(FieldAccessEventType.EXACT_MATCH_FILTER).withOutcome(FieldAccessOutcome.MATCH)
+         .withTableName("reading").withFieldNames(List.of("notes")).withActionName("query").withRecordCount(1)
+         .withSearchValueFingerprints(List.of(runtime.resolver().fingerprintSearchValue("known"))));
+      assertThat(logs()).as("the search is queued, not yet written").extracting(row -> row.getValueString("event_type")).doesNotContain("EXACT_MATCH_FILTER");
+   }
+
+
+
+   /*******************************************************************************
+    ** Run the operation as the person on another thread and return once it is
+    ** parked at its first reading of the clock, which both operations make
+    ** inside their synchronized block.
+    *******************************************************************************/
+   private <T> Future<T> parkInside(Callable<T> operation) throws Exception
+   {
+      CLOCK.beforeInstant = () ->
+      {
+         if(Thread.currentThread() == working.get() && parked.getCount() == 1)
+         {
+            heldTheRuntimesMonitor.set(Thread.holdsLock(runtime));
+            parked.countDown();
+            try
+            {
+               resume.await();
+            }
+            catch(InterruptedException e)
+            {
+               Thread.currentThread().interrupt();
+            }
+         }
+      };
+      Future<T> future = threads.submit(() ->
+      {
+         QContext.init(instance, person);
+         try
+         {
+            working.set(Thread.currentThread());
+            return (operation.call());
+         }
+         finally
+         {
+            QContext.clear();
+         }
+      });
+      assertThat(parked.await(WAIT_SECONDS, TimeUnit.SECONDS)).as("the operation read the clock").isTrue();
+      assertThat(heldTheRuntimesMonitor).as("the operation is parked holding the runtime's monitor").isTrue();
+      return (future);
+   }
+
+
+
+   /*******************************************************************************
+    ** While the operation is parked: the access-log writer drains its queue (what
+    ** its worker does on every tick), and a thread that holds no lock is told
+    ** the classification of a field and the active retention rules.
+    *******************************************************************************/
+   private void othersFinishMeanwhile() throws Exception
+   {
+      Future<?> writer = threads.submit(() -> runtime.logger().flush());
+      Future<DataClassification> question = threads.submit(() ->
+      {
+         QContext.init(instance, person);
+         try
+         {
+            runtime.activeRetentionRules();
+            return (SecuredDataClassifications.effective(reading, "notes"));
+         }
+         finally
+         {
+            QContext.clear();
+         }
+      });
+
+      try
+      {
+         writer.get(WAIT_SECONDS, TimeUnit.SECONDS);
+         assertThat(question.get(WAIT_SECONDS, TimeUnit.SECONDS)).isEqualTo(DataClassification.PII);
+      }
+      catch(TimeoutException e)
+      {
+         abandon();
+         fail("A thread waited " + WAIT_SECONDS + " seconds for the monitor the parked operation holds (writer done: " + writer.isDone() + ", question done: " + question.isDone()
+            + "): the operation needs the lock that thread holds next, so neither would ever finish");
+      }
+   }
+
+
+
+   /*******************************************************************************
+    ** Threads of this runtime are stuck holding its locks: closing it would wait
+    ** for them, so it is left to the daemon threads instead of being closed.
+    *******************************************************************************/
+   private void abandon()
+   {
+      SecuredDataTestSupport.RUNTIMES.remove(runtime);
+   }
+}

src/test/java/com/kingsrook/qbits/secureddata/SecuredDataRuleReloadTest.java

@@ -0,0 +1,682 @@
+/*
+ * QQQ - Low-code Application Framework for Engineers.
+ * Copyright (C) 2021-2025.  Kingsrook, LLC
+ * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States
+ * contact@kingsrook.com
+ * https://github.com/Kingsrook/
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.kingsrook.qbits.secureddata;
+
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
+import com.kingsrook.qbits.secureddata.metadata.SecuredDataTables;
+import com.kingsrook.qbits.secureddata.retention.RetentionRules;
+import com.kingsrook.qbits.secureddata.support.SystemWrites;
+import com.kingsrook.qqq.backend.core.actions.QBackendTransaction;
+import com.kingsrook.qqq.backend.core.actions.ai.DataClassification;
+import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizerInterface;
+import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizers;
+import com.kingsrook.qqq.backend.core.actions.permissions.fieldaccess.FieldAccessDecision;
+import com.kingsrook.qqq.backend.core.actions.tables.QueryAction;
+import com.kingsrook.qqq.backend.core.actions.tables.query.VirtualFieldValueProviderInterface;
+import com.kingsrook.qqq.backend.core.context.QContext;
+import com.kingsrook.qqq.backend.core.exceptions.QException;
+import com.kingsrook.qqq.backend.core.model.actions.tables.QueryOrGetInputInterface;
+import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput;
+import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator;
+import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria;
+import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter;
+import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput;
+import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput;
+import com.kingsrook.qqq.backend.core.model.data.QRecord;
+import com.kingsrook.qqq.backend.core.model.metadata.QInstance;
+import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference;
+import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData;
+import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType;
+import com.kingsrook.qqq.backend.core.model.metadata.fields.QVirtualFieldMetaData;
+import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
+import com.kingsrook.qqq.backend.core.model.session.QSystemUserSession;
+import com.kingsrook.qqq.backend.core.modules.backend.implementations.memory.MemoryRecordStore;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.classification;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.close;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.policyRow;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.setup;
+import static com.kingsrook.qbits.secureddata.SecuredDataTestSupport.user;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+
+/*******************************************************************************
+ ** Reading rule rows (classifications, retention rules, policies) is a query on
+ ** a secured-data table, and other code runs inside that query: a virtual field
+ ** value provider or a post-query customizer that another QBit or the host put
+ ** on the table. When that code runs a query with filter criteria, core asks
+ ** this QBit for a classification - which needs the rows being read.
+ **
+ ** These tests put such code on the rule tables and prove: the read asks for
+ ** stored fields only, so no value provider runs inside it; a question asked
+ ** inside a read is answered from the rows held before and starts no other
+ ** read; only the reading thread, as the system, can be given that answer; and
+ ** a read that fails is tried again by the next question.
+ *******************************************************************************/
+class SecuredDataRuleReloadTest
+{
+   QInstance          instance;
+   QTableMetaData     person;
+   SecuredDataRuntime runtime;
+
+
+
+   /*******************************************************************************
+    ** Something to do inside a query on a rule table.
+    *******************************************************************************/
+   @FunctionalInterface
+   interface NestedWork
+   {
+      /*******************************************************************************
+       ** Do it.
+       *******************************************************************************/
+      void run() throws Exception;
+   }
+
+
+
+   /*******************************************************************************
+    ** Stands in for another QBit's virtual field on a secured-data table: filling
+    ** it runs a query that has filter criteria.
+    *******************************************************************************/
+   public static class NestedQueryProvider implements VirtualFieldValueProviderInterface
+   {
+      static final AtomicInteger RUNS = new AtomicInteger();
+
+
+
+      /*******************************************************************************
+       ** Fill the field from a query with criteria, as a real provider does.
+       *******************************************************************************/
+      @Override
+      public void fill(QTableMetaData table, String fieldName, List<QRecord> records, QueryInput input) throws QException
+      {
+         RUNS.incrementAndGet();
+         queryWithCriteria();
+         records.forEach(record -> record.setValue(fieldName, "filled"));
+      }
+   }
+
+
+
+   /*******************************************************************************
+    ** A post-query customizer on a secured-data table. Core runs it for every
+    ** query on the table, whatever fields the query names.
+    *******************************************************************************/
+   public static class NestedWorkCustomizer implements TableCustomizerInterface
+   {
+      static final AtomicInteger RUNS = new AtomicInteger();
+      static volatile NestedWork work;
+
+
+
+      /*******************************************************************************
+       ** Run the work the test asked for, inside the query.
+       *******************************************************************************/
+      @Override
+      public List<QRecord> postQuery(QueryOrGetInputInterface queryInput, List<QRecord> records) throws QException
+      {
+         NestedWork toRun = work;
+         if(toRun != null)
+         {
+            RUNS.incrementAndGet();
+            try
+            {
+               toRun.run();
+            }
+            catch(QException | RuntimeException e)
+            {
+               throw (e);
+            }
+            catch(Exception e)
+            {
+               throw (new QException("Nested work failed", e));
+            }
+         }
+         return (records);
+      }
+   }
+
+
+
+   /*******************************************************************************
+    ** A person table with a date, so a retention rule on it is valid.
+    *******************************************************************************/
+   @BeforeEach
+   void init() throws Exception
+   {
+      instance = setup();
+      person = instance.getTable("person");
+      person.withField(new QFieldMetaData("createDate", QFieldType.DATE_TIME).withBackendName("createDate"));
+      runtime = SecuredDataRuntime.current();
+      NestedQueryProvider.RUNS.set(0);
+      NestedWorkCustomizer.RUNS.set(0);
+      NestedWorkCustomizer.work = null;
+   }
+
+
+
+   /*******************************************************************************
+    ** Cleanup.
+    *******************************************************************************/
+   @AfterEach
+   void cleanup()
+   {
+      NestedWorkCustomizer.work = null;
+      close();
+   }
+
+
+
+   /*******************************************************************************
+    ** A query with filter criteria on a host table: core validates the operand
+    ** of a classified field, so it asks for the field's classification - and
+    ** this QBit then asks whether a retention rule names the field.
+    *******************************************************************************/
+   private static void queryWithCriteria() throws QException
+   {
+      new QueryAction().execute(new QueryInput("person").withFilter(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 1))));
+   }
+
+
+
+   /*******************************************************************************
+    ** Another QBit's virtual field on a secured-data table.
+    *******************************************************************************/
+   private void addProvider(String table)
+   {
+      instance.getTable(table).addVirtualField(new QVirtualFieldMetaData("labels", QFieldType.STRING).withIsQuerySelectable(true)
+         .withValueProvider(new QCodeReference(NestedQueryProvider.class)));
+   }
+
+
+
+   /*******************************************************************************
+    ** A post-query customizer on a secured-data table.
+    *******************************************************************************/
+   private void addCustomizer(String table)
+   {
+      instance.getTable(table).withCustomizer(TableCustomizers.POST_QUERY_RECORD, new QCodeReference(NestedWorkCustomizer.class));
+   }
+
+
+
+   /*******************************************************************************
+    ** A row as another node writes it: straight into the store. None of this
+    ** node's customizers run and nothing here reads the rule rows; this node
+    ** learns of the change when its version moves (the cluster event).
+    *******************************************************************************/
+   private static QRecord writtenElsewhere(String table, QRecord row) throws QException
+   {
+      return (MemoryRecordStore.getInstance().insert(new InsertInput(table).withRecord(row), true).getFirst());
+   }
+
+
+
+   /*******************************************************************************
+    ** A classification of a person field, written by another node.
+    *******************************************************************************/
+   private QRecord classifiedElsewhere(String field, String classification) throws QException
+   {
+      QRecord row = writtenElsewhere(SecuredDataTables.CLASSIFICATION, new QRecord().withValue("table_name", "person")
+         .withValue("field_name", field).withValue("classification", classification));
+      runtime.invalidateClassifications();
+      return (row);
+   }
+
+
+
+   /*******************************************************************************
+    ** A classification row changed by another node.
+    *******************************************************************************/
+   private void reclassifiedElsewhere(QRecord row, String classification) throws QException
+   {
+      MemoryRecordStore.getInstance().update(new UpdateInput(SecuredDataTables.CLASSIFICATION).withRecord(new QRecord()
+         .withValue("id", row.getValue("id")).withValue("classification", classification)), false);
+      runtime.invalidateClassifications();
+   }
+
+
+
+   /*******************************************************************************
+    ** An active retention rule on one field of the person table, written by
+    ** another node.
+    *******************************************************************************/
+   private void ruleWrittenElsewhere(String field) throws QException
+   {
+      writtenElsewhere(SecuredDataTables.RETENTION_RULE, new QRecord().withValue("name", "30 days: " + field).withValue("table_name", "person")
+         .withValue("field_names", field).withValue("age_field_name", "createDate").withValue("age_days", 30).withValue("action", "NULL_WITH_MARKER")
+         .withValue("audit_treatment", "SHREDDABLE").withValue("is_active", true));
+      runtime.invalidateRetentionRules();
+   }
+
+
+
+   /*******************************************************************************
+    ** The classification of a person field.
+    *******************************************************************************/
+   private DataClassification classificationOf(String field)
+   {
+      return (SecuredDataClassifications.effective(person, field));
+   }
+
+
+
+   /*******************************************************************************
+    ** The reported failure: a value provider on the classification table whose
+    ** work is a query with criteria, and one classification row. Before the fix
+    ** the next classification question recursed until the stack overflowed, and
+    ** so did every question after it. Now the rule read names its stored fields,
+    ** so the provider does not run inside it at all.
+    *******************************************************************************/
+   @Test
+   void aValueProviderOnTheClassificationTableDoesNotRunInsideTheRuleRead() throws Exception
+   {
+      addProvider(SecuredDataTables.CLASSIFICATION);
+      classification("notes", "PHI");
+
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PHI);
+      queryWithCriteria();
+      assertThat(NestedQueryProvider.RUNS).hasValue(0);
+      assertThat(SystemWrites.query(SecuredDataTables.CLASSIFICATION, new QQueryFilter())).singleElement()
+         .satisfies(row -> assertThat(row.getValues()).doesNotContainKey("labels").containsEntry("field_name", "notes"));
+   }
+
+
+
+   /*******************************************************************************
+    ** The virtual field is still there for a query that asks for every field (a
+    ** screen listing the classifications). Its provider's query with criteria
+    ** asks for a classification while the rows are out of date: the rows are
+    ** read once, inside the provider, and nothing recurses.
+    *******************************************************************************/
+   @Test
+   void anOrdinaryQueryOnTheClassificationTableStillFillsTheVirtualField() throws Exception
+   {
+      addProvider(SecuredDataTables.CLASSIFICATION);
+      classifiedElsewhere("notes", "PHI");
+
+      List<QRecord> rows = SystemWrites.call(() -> new QueryAction().execute(new QueryInput(SecuredDataTables.CLASSIFICATION)).getRecords());
+      assertThat(rows).singleElement().satisfies(row -> assertThat(row.getValueString("labels")).isEqualTo("filled"));
+      assertThat(NestedQueryProvider.RUNS).hasValue(1);
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PHI);
+   }
+
+
+
+   /*******************************************************************************
+    ** A post-query customizer runs inside the rule read whatever fields the read
+    ** names. Its query with criteria asks for a classification in the middle of
+    ** the read of the classifications: before the fix that started the read again
+    ** and recursed until the stack overflowed. Now the rows are read once.
+    *******************************************************************************/
+   @Test
+   void aQuestionAskedInsideTheClassificationReadDoesNotStartAnotherRead() throws Exception
+   {
+      addCustomizer(SecuredDataTables.CLASSIFICATION);
+      classifiedElsewhere("notes", "PHI");
+
+      NestedWorkCustomizer.work = SecuredDataRuleReloadTest::queryWithCriteria;
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PHI);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(1);
+
+      assertThat(classificationOf("email")).isEqualTo(DataClassification.PII);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(1);
+   }
+
+
+
+   /*******************************************************************************
+    ** What a question asked inside the read is told, and who can ask it.
+    **
+    ** The nested question gets the rows this node held when the read began (no
+    ** rows, the first time; then the rows of the previous version), in both
+    ** directions of a change. It can only be asked by the thread doing the read,
+    ** which is inside the QBit's system read: the session is the system session
+    ** and the resolver answers FULL there without looking at any classification,
+    ** so no person's field access is decided from that answer. The question that
+    ** started the read gets the new rows.
+    *******************************************************************************/
+   @Test
+   void aQuestionAskedInsideTheReadIsAnsweredFromTheRowsHeldBefore() throws Exception
+   {
+      addCustomizer(SecuredDataTables.CLASSIFICATION);
+      assertThat(classificationOf("notes")).isNull();
+
+      List<DataClassification> nestedAnswers = new ArrayList<>();
+      List<Boolean> nestedAsSystem = new ArrayList<>();
+      List<FieldAccessDecision> nestedDecisions = new ArrayList<>();
+      NestedWork ask = () ->
+      {
+         nestedAnswers.add(classificationOf("notes"));
+         nestedAsSystem.add(QContext.getQSession() instanceof QSystemUserSession && SystemWrites.active());
+         nestedDecisions.add(runtime.resolver().resolve(person, person.getField("notes")));
+      };
+
+      /////////////////////////////////////////////////////////////////////////
+      // a row raises the field: inside the read it is still unclassified,   //
+      // and the question that started the read is answered PHI              //
+      /////////////////////////////////////////////////////////////////////////
+      QRecord row = classifiedElsewhere("notes", "PHI");
+      NestedWorkCustomizer.work = ask;
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PHI);
+      NestedWorkCustomizer.work = null;
+      assertThat(nestedAnswers).containsExactly((DataClassification) null);
+
+      /////////////////////////////////////////////////////////////////////////
+      // the row is changed to lower the field: inside the read it is still  //
+      // PHI, and the question that started the read is answered PUBLIC      //
+      /////////////////////////////////////////////////////////////////////////
+      reclassifiedElsewhere(row, "PUBLIC");
+      NestedWorkCustomizer.work = ask;
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PUBLIC);
+      NestedWorkCustomizer.work = null;
+      assertThat(nestedAnswers).containsExactly(null, DataClassification.PHI);
+
+      assertThat(nestedAsSystem).containsExactly(true, true);
+      assertThat(nestedDecisions).containsExactly(FieldAccessDecision.FULL, FieldAccessDecision.FULL);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(2);
+   }
+
+
+
+   /*******************************************************************************
+    ** A request on another thread is never given the rows held before: while the
+    ** read is running it waits, and then it is answered from the rows the read
+    ** returned.
+    *******************************************************************************/
+   @Test
+   void aRequestOnAnotherThreadWaitsForTheReadAndIsGivenItsRows() throws Exception
+   {
+      addCustomizer(SecuredDataTables.CLASSIFICATION);
+      QRecord row = classifiedElsewhere("notes", "PUBLIC");
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PUBLIC);
+      reclassifiedElsewhere(row, "PHI");
+
+      CountDownLatch readIsRunning = new CountDownLatch(1);
+      CountDownLatch letReadFinish = new CountDownLatch(1);
+      NestedWorkCustomizer.work = () ->
+      {
+         readIsRunning.countDown();
+         assertThat(letReadFinish.await(30, TimeUnit.SECONDS)).isTrue();
+      };
+
+      ExecutorService pool = Executors.newFixedThreadPool(2);
+      try
+      {
+         Future<DataClassification> reader = pool.submit(asUser("u2", () -> classificationOf("notes")));
+         assertThat(readIsRunning.await(30, TimeUnit.SECONDS)).isTrue();
+
+         Future<DataClassification> other = pool.submit(asUser("u3", () -> classificationOf("notes")));
+         assertThatThrownBy(() -> other.get(500, TimeUnit.MILLISECONDS)).isInstanceOf(TimeoutException.class);
+
+         letReadFinish.countDown();
+         assertThat(reader.get(30, TimeUnit.SECONDS)).isEqualTo(DataClassification.PHI);
+         assertThat(other.get(30, TimeUnit.SECONDS)).isEqualTo(DataClassification.PHI);
+         assertThat(NestedWorkCustomizer.RUNS).hasValue(1);
+      }
+      finally
+      {
+         letReadFinish.countDown();
+         pool.shutdownNow();
+      }
+   }
+
+
+
+   /*******************************************************************************
+    ** A person's request on its own thread.
+    *******************************************************************************/
+   private <T> Callable<T> asUser(String id, Callable<T> work)
+   {
+      return (() ->
+      {
+         QContext.init(instance, user(id));
+         try
+         {
+            return (work.call());
+         }
+         finally
+         {
+            QContext.clear();
+         }
+      });
+   }
+
+
+
+   /*******************************************************************************
+    ** A read that fails (here, in code running inside it, after that code asked a
+    ** nested question) throws to the request that started it and leaves nothing
+    ** behind: the next request reads again and gets the current rows - not the
+    ** rows held before, and not another failure. An Error, such as the stack
+    ** overflow this defect used to end in, is no different.
+    *******************************************************************************/
+   @Test
+   void aClassificationReadThatFailsIsTriedAgainByTheNextQuestion() throws Exception
+   {
+      addCustomizer(SecuredDataTables.CLASSIFICATION);
+      QRecord row = classifiedElsewhere("notes", "PUBLIC");
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PUBLIC);
+      reclassifiedElsewhere(row, "PHI");
+
+      NestedWorkCustomizer.work = () ->
+      {
+         classificationOf("notes");
+         throw (new QException("The store is unavailable"));
+      };
+      assertThatThrownBy(() -> classificationOf("notes")).isInstanceOf(IllegalStateException.class);
+
+      NestedWorkCustomizer.work = () ->
+      {
+         classificationOf("notes");
+         throw (new StackOverflowError());
+      };
+      assertThatThrownBy(() -> classificationOf("notes")).isInstanceOf(StackOverflowError.class);
+
+      NestedWorkCustomizer.work = null;
+      assertThat(classificationOf("notes")).isEqualTo(DataClassification.PHI);
+   }
+
+
+
+   /*******************************************************************************
+    ** The active retention rules have the same shape: core's classification
+    ** question for a host field also asks whether a retention rule names the
+    ** field. A value provider on the retention rule table, with one active rule,
+    ** recursed the same way before the fix; it does not run inside the rule read.
+    *******************************************************************************/
+   @Test
+   void aValueProviderOnTheRetentionRuleTableDoesNotRunInsideTheRuleRead() throws Exception
+   {
+      addProvider(SecuredDataTables.RETENTION_RULE);
+      ruleWrittenElsewhere("notes");
+
+      assertThat(RetentionRules.active("person", "notes")).hasSize(1);
+      queryWithCriteria();
+      assertThat(NestedQueryProvider.RUNS).hasValue(0);
+   }
+
+
+
+   /*******************************************************************************
+    ** A question asked inside the read of the retention rules starts no other
+    ** read and is answered from the rules held before; the question that started
+    ** the read gets the new rules. A read that fails is tried again.
+    *******************************************************************************/
+   @Test
+   void aQuestionAskedInsideTheRetentionRuleReadIsAnsweredFromTheRulesHeldBefore() throws Exception
+   {
+      addCustomizer(SecuredDataTables.RETENTION_RULE);
+      assertThat(RetentionRules.active("person", "notes")).isEmpty();
+      ruleWrittenElsewhere("notes");
+
+      List<Integer> nestedAnswers = new ArrayList<>();
+      NestedWorkCustomizer.work = () ->
+      {
+         queryWithCriteria();
+         nestedAnswers.add(RetentionRules.active("person", "notes").size());
+         if(nestedAnswers.size() == 1)
+         {
+            throw (new QException("The store is unavailable"));
+         }
+      };
+      assertThatThrownBy(() -> RetentionRules.active("person", "notes")).isInstanceOf(IllegalStateException.class);
+      assertThat(RetentionRules.active("person", "notes")).hasSize(1);
+      assertThat(nestedAnswers).containsExactly(0, 0);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(2);
+
+      assertThat(RetentionRules.active("person", "notes")).hasSize(1);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(2);
+   }
+
+
+
+   /*******************************************************************************
+    ** A transaction that edited a rule reads the rules through itself, not from
+    ** the rules the node keeps. A question asked inside that read is answered
+    ** from the rules every thread shares: before the fix it read the edits again,
+    ** and recursed until the stack overflowed.
+    *******************************************************************************/
+   @Test
+   void aQuestionAskedInsideAReadOfATransactionsOwnEditsDoesNotReadThemAgain() throws Exception
+   {
+      addCustomizer(SecuredDataTables.RETENTION_RULE);
+      ruleWrittenElsewhere("notes");
+      assertThat(RetentionRules.active("person", "notes")).hasSize(1);
+
+      QContext.setTransaction(new QBackendTransaction());
+      RetentionRules.editedInTransaction();
+      try
+      {
+         NestedWorkCustomizer.work = SecuredDataRuleReloadTest::queryWithCriteria;
+         assertThat(RetentionRules.active("person", "notes")).hasSize(1);
+         assertThat(NestedWorkCustomizer.RUNS).hasValue(1);
+      }
+      finally
+      {
+         ////////////////////////////////////////////////////////////////////////
+         // the next question outside a transaction ends this thread's edits  //
+         ////////////////////////////////////////////////////////////////////////
+         NestedWorkCustomizer.work = null;
+         QContext.setTransaction(null);
+         RetentionRules.active("person", "notes");
+      }
+   }
+
+
+
+   /*******************************************************************************
+    ** The policies are read the same way (rows first, version afterwards), so
+    ** they are guarded the same way: code inside the policy read that asks the
+    ** resolver a policy question starts no other read, and a read that fails is
+    ** tried again.
+    *******************************************************************************/
+   @Test
+   void aQuestionAskedInsideThePolicyReadDoesNotStartAnotherRead() throws Exception
+   {
+      addCustomizer(SecuredDataTables.POLICY);
+      addProvider(SecuredDataTables.POLICY);
+      assertThat(runtime.resolver().revealPolicies(person, "email")).isEmpty();
+      writtenElsewhere(SecuredDataTables.POLICY, policyRow("cs_admin", "PII", "REVEALABLE"));
+      runtime.invalidatePolicies();
+
+      List<Integer> nestedAnswers = new ArrayList<>();
+      NestedWorkCustomizer.work = () ->
+      {
+         nestedAnswers.add(runtime.resolver().revealPolicies(person, "email").size());
+         if(nestedAnswers.size() == 1)
+         {
+            throw (new QException("The store is unavailable"));
+         }
+      };
+      assertThatThrownBy(() -> runtime.resolver().revealPolicies(person, "email")).isInstanceOf(IllegalStateException.class);
+      assertThat(runtime.resolver().revealPolicies(person, "email")).hasSize(1);
+      assertThat(nestedAnswers).containsExactly(0, 0);
+      assertThat(NestedQueryProvider.RUNS).hasValue(0);
+
+      NestedWorkCustomizer.RUNS.set(0);
+      assertThat(runtime.resolver().revealPolicies(person, "email")).hasSize(1);
+      assertThat(NestedWorkCustomizer.RUNS).hasValue(0);
+   }
+
+
+
+   /*******************************************************************************
+    ** The rows themselves, without a table: nothing is held before the first
+    ** read, a nested question never reads, a failed read is forgotten, and a new
+    ** version is read once.
+    *******************************************************************************/
+   @Test
+   void ruleRowsAreReadOncePerVersionAndNeverFromInsideTheirOwnRead()
+   {
+      RuleRows rows = new RuleRows();
+      AtomicInteger reads = new AtomicInteger();
+      List<QRecord> first = List.of(new QRecord().withValue("id", 1));
+      List<QRecord> second = List.of(new QRecord().withValue("id", 2));
+
+      assertThatThrownBy(() -> rows.get(1, () ->
+      {
+         reads.incrementAndGet();
+         assertThat(rows.get(1, () -> fail("a nested question must not read"))).isEmpty();
+         throw (new IllegalStateException("The store is unavailable"));
+      })).isInstanceOf(IllegalStateException.class);
+
+      assertThat(rows.get(1, () ->
+      {
+         reads.incrementAndGet();
+         assertThat(rows.get(1, () -> fail("a nested question must not read"))).isEmpty();
+         return (first);
+      })).isEqualTo(first);
+      assertThat(rows.get(1, () -> fail("rows of this version are held"))).isEqualTo(first);
+
+      assertThat(rows.get(2, () ->
+      {
+         reads.incrementAndGet();
+         assertThat(rows.get(2, () -> fail("a nested question must not read"))).isEqualTo(first);
+         return (second);
+      })).isEqualTo(second);
+      assertThat(reads).hasValue(3);
+   }
+
+
+
+   /*******************************************************************************
+    ** A read that must not happen.
+    *******************************************************************************/
+   private static List<QRecord> fail(String why)
+   {
+      throw (new AssertionError(why));
+   }
+}

src/test/java/com/kingsrook/qbits/secureddata/SecuredDataTestSupport.java

@@ -234,6 +234,7 @@ final class SecuredDataTestSupport
     *******************************************************************************/
    static void close()
    {
+      CLOCK.beforeInstant = null;
       RUNTIMES.forEach(SecuredDataRuntime::close);
       RUNTIMES.clear();
       QContext.clear();
@@ -248,6 +249,9 @@ final class SecuredDataTestSupport
    {
       Instant now = NOW;
 
+      /** Run on the reading thread before every reading of the clock; a test parks a thread at a known point with it. */
+      volatile Runnable beforeInstant;
+
 
 
       /*******************************************************************************
@@ -275,6 +279,11 @@ final class SecuredDataTestSupport
        *******************************************************************************/
       public Instant instant()
       {
+         Runnable hook = beforeInstant;
+         if(hook != null)
+         {
+            hook.run();
+         }
          return (now);
       }
    }