fix: a reveal and the access-log writer no longer wait for each other #45

2026-11-14T09:42:36Z

james.maes commented

2026-11-14 09:42:36 +00:00

The defect

A reveal can deadlock with the access-log writer. When it does, the node stops answering: every later request blocks too, because every request asks this QBit for a classification.

Seen in a host on PostgreSQL. A person read a record with a masked field and revealed it straight away. The read queued access-log rows; the writer's tick drained them while the reveal was running. Thread dump (Found one Java-level deadlock):

request thread (reveal)
  SystemWrites.insertOne                        waiting for the anomaly counters' monitor
  RevealService.reveal                          holds the runtime's monitor

secured-data-access-log (the writer)
  SecuredDataRuntime.classificationRows         waiting for the runtime's monitor
  SecuredDataClassifications.row / effective
  SecuredDataAuditProtector.classificationOf
  core ClassifiedValues.isClassified
  core ValueBehaviorApplier.validateClassifiedCriteriaValues   <- a query with criteria ...
  core AuditAction.fetchIdFromName                             <- ... made by the audit of ...
  core InsertAction
  SystemWrites.insertOne                        holds the anomaly counters' monitor   <- ... its own insert
  SecuredDataAccessLogger.flush

every other thread
  SecuredDataRuntime.classificationRows         waiting for the runtime's monitor

RevealService.reveal and BreakGlassService.start, end and expire 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). classificationRows() and activeRetentionRules() were synchronized on the same monitor, and core calls them from everywhere: 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 any thread that asked while holding another lock of this QBit took the two locks in the opposite order to a reveal:

Thread	Holds	Then needs
reveal, break-glass start/end/expiry	runtime	anomaly counters (its own access-log row), resolver, key source
access-log writer	anomaly counters	runtime (a classification, asked inside its insert)
a request computing its policies	resolver	runtime (a classification, asked by its break-glass query)
an audited write	key source	runtime (a classification, asked by the key lookup)

The first two rows are the thread dump above. The last two follow from reading the code and were not reproduced.

The inversion is not new: the same synchronized structure is in 0.1.0-RC.5, RC.6 and RC.7. It needs a reveal or a break-glass change to be running, so it shows only once fields are masked and people reveal them.

The fix

The rule rows a node keeps (classifications, active retention rules) have a lock of their own, ruleLock, held only while they are answered or read. classificationRows() and activeRetentionRules() no longer take the runtime's monitor.

A read under ruleLock is a query made as the system. It takes none of the QBit's other locks (no access-log insert, no resolver state, no key lookup), so nothing that holds ruleLock ever waits for one of them. A question asked from inside the read is on the reading thread and re-enters, as before (RuleRows).
The runtime's monitor is left to what it was for: one reveal or break-glass change at a time. Nothing takes it while holding another lock of the QBit.
The resulting order is: runtime, the writer's flush lock, anomaly counters or resolver, key source, rule lock.

RuleRows is unchanged apart from its comment: it still requires its caller to hold the owner's lock, which for classifications and retention rules is now ruleLock.

Tests

SecuredDataLockOrderTest, on a relational database (H2 in PostgreSQL mode with the audit tables, the setup the retention tests already use), where an insert really writes its audit:

a thread is parked inside a reveal, holding the runtime's monitor (the test asserts that it does); meanwhile the access-log writer must drain a queued search of a classified field, and another thread must be told a classification and the active retention rules; then the reveal must finish and return the value;
the same with a thread parked inside the start of a break-glass session.

Both fail without the fix: neither the writer nor the question finishes within the wait (writer done: false, question done: false). The test clock got a hook (MutableClock.beforeInstant) to park a thread at a known point.

Not covered here: the pool-level case in which every database connection is held by a thread waiting for ruleLock while the reader waits for a connection. Reading under a lock has had that property since the rows were first cached; it is unchanged.

Verification

./mvnw verify on this head (a366e8b): 114 tests, 0 failures, 0 Checkstyle violations, coverage checks met, BUILD SUCCESS. This repository builds against QQQ 4.1.0-RC.26.

A port to release/0.1 follows as its own pull request.

🤖 Generated with Claude Code

## The defect A reveal can deadlock with the access-log writer. When it does, the node stops answering: every later request blocks too, because every request asks this QBit for a classification. Seen in a host on PostgreSQL. A person read a record with a masked field and revealed it straight away. The read queued access-log rows; the writer's tick drained them while the reveal was running. Thread dump (`Found one Java-level deadlock`): request thread (reveal) SystemWrites.insertOne waiting for the anomaly counters' monitor RevealService.reveal holds the runtime's monitor secured-data-access-log (the writer) SecuredDataRuntime.classificationRows waiting for the runtime's monitor SecuredDataClassifications.row / effective SecuredDataAuditProtector.classificationOf core ClassifiedValues.isClassified core ValueBehaviorApplier.validateClassifiedCriteriaValues <- a query with criteria ... core AuditAction.fetchIdFromName <- ... made by the audit of ... core InsertAction SystemWrites.insertOne holds the anomaly counters' monitor <- ... its own insert SecuredDataAccessLogger.flush every other thread SecuredDataRuntime.classificationRows waiting for the runtime's monitor `RevealService.reveal` and `BreakGlassService.start`, `end` and `expire` 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). `classificationRows()` and `activeRetentionRules()` were synchronized on the same monitor, and core calls them from everywhere: 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 any thread that asked while holding another lock of this QBit took the two locks in the opposite order to a reveal: | Thread | Holds | Then needs | |---|---|---| | reveal, break-glass start/end/expiry | runtime | anomaly counters (its own access-log row), resolver, key source | | access-log writer | anomaly counters | runtime (a classification, asked inside its insert) | | a request computing its policies | resolver | runtime (a classification, asked by its break-glass query) | | an audited write | key source | runtime (a classification, asked by the key lookup) | The first two rows are the thread dump above. The last two follow from reading the code and were not reproduced. The inversion is not new: the same `synchronized` structure is in 0.1.0-RC.5, RC.6 and RC.7. It needs a reveal or a break-glass change to be running, so it shows only once fields are masked and people reveal them. ## The fix The rule rows a node keeps (classifications, active retention rules) have a lock of their own, `ruleLock`, held only while they are answered or read. `classificationRows()` and `activeRetentionRules()` no longer take the runtime's monitor. - A read under `ruleLock` is a query made as the system. It takes none of the QBit's other locks (no access-log insert, no resolver state, no key lookup), so nothing that holds `ruleLock` ever waits for one of them. A question asked from inside the read is on the reading thread and re-enters, as before (`RuleRows`). - The runtime's monitor is left to what it was for: one reveal or break-glass change at a time. Nothing takes it while holding another lock of the QBit. - The resulting order is: runtime, the writer's flush lock, anomaly counters or resolver, key source, rule lock. `RuleRows` is unchanged apart from its comment: it still requires its caller to hold the owner's lock, which for classifications and retention rules is now `ruleLock`. ## Tests `SecuredDataLockOrderTest`, on a relational database (H2 in PostgreSQL mode with the audit tables, the setup the retention tests already use), where an insert really writes its audit: - a thread is parked inside a reveal, holding the runtime's monitor (the test asserts that it does); meanwhile the access-log writer must drain a queued search of a classified field, and another thread must be told a classification and the active retention rules; then the reveal must finish and return the value; - the same with a thread parked inside the start of a break-glass session. Both fail without the fix: neither the writer nor the question finishes within the wait (`writer done: false, question done: false`). The test clock got a hook (`MutableClock.beforeInstant`) to park a thread at a known point. Not covered here: the pool-level case in which every database connection is held by a thread waiting for `ruleLock` while the reader waits for a connection. Reading under a lock has had that property since the rows were first cached; it is unchanged. ## Verification `./mvnw verify` on this head (`a366e8b`): 114 tests, 0 failures, 0 Checkstyle violations, coverage checks met, BUILD SUCCESS. This repository builds against QQQ 4.1.0-RC.26. A port to `release/0.1` follows as its own pull request. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 1 commit 2026-11-14 09:42:37 +00:00

fix: a reveal and the access-log writer no longer wait for each other

maven / build (push) Successful in 2m34s

Details

build / maven (push) Successful in 2m34s

Details

a366e8b96e

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>

james.maes referenced this pull request

2026-11-14 09:43:02 +00:00

fix: a reveal and the access-log writer no longer wait for each other (release/0.1 port of #45) #46

All checks were successful

maven / build (push) Successful in 2m34s

Details

build / maven (push) Successful in 2m34s

Details

You are not authorized to merge this pull request.

This pull request can be merged automatically.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/reveal-access-log-deadlock:fix/reveal-access-log-deadlock

git checkout fix/reveal-access-log-deadlock

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qbit-secured-data#45