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

Open
james.maes wants to merge 1 commits from fix/reveal-access-log-deadlock-0.1 into release/0.1
pull from: fix/reveal-access-log-deadlock-0.1
Owner

Port of #45 to the 0.1 release line: the same commit, cherry-picked onto release/0.1 (the two branches differ only in the version in pom.xml). It is a pull request of its own, like #44, so that it can be reviewed and released without waiting for a merge of develop. Merge #45 first.

The defect

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

RevealService.reveal and BreakGlassService.start, end and expire hold the runtime's monitor for their whole operation. classificationRows() and activeRetentionRules() were synchronized on the same monitor, and core calls them for every field it decides on and for every filter criterion of every query, including the queries made inside this QBit's own writes. The access-log writer holds the anomaly counters while it inserts a row, and that insert asks for a classification; 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 with 0.1.0-RC.7, when a person read a record with a masked field and revealed it straight away (thread dump in #45). The same structure is in 0.1.0-RC.5 and RC.6.

The fix

The rule rows a node keeps (classifications, active retention rules) have a lock of their own, 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 the rule lock waits for one. The runtime's monitor is left to serialising reveals and break-glass changes.

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; then the parked operation must finish. Both tests fail without the fix.

Verification

./mvnw verify on this head (c2fde97): 114 tests, 0 failures, 0 Checkstyle violations, coverage checks met, BUILD SUCCESS.

🤖 Generated with Claude Code

Port of #45 to the 0.1 release line: the same commit, cherry-picked onto `release/0.1` (the two branches differ only in the version in `pom.xml`). It is a pull request of its own, like #44, so that it can be reviewed and released without waiting for a merge of develop. Merge #45 first. ## The defect A reveal can deadlock with the access-log writer, and then the node stops answering: every later request blocks, because every request asks this QBit for a classification. `RevealService.reveal` and `BreakGlassService.start`, `end` and `expire` hold the runtime's monitor for their whole operation. `classificationRows()` and `activeRetentionRules()` were synchronized on the same monitor, and core calls them for every field it decides on and for every filter criterion of every query, including the queries made inside this QBit's own writes. The access-log writer holds the anomaly counters while it inserts a row, and that insert asks for a classification; 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 with 0.1.0-RC.7, when a person read a record with a masked field and revealed it straight away (thread dump in #45). The same structure is in 0.1.0-RC.5 and RC.6. ## The fix The rule rows a node keeps (classifications, active retention rules) have a lock of their own, 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 the rule lock waits for one. The runtime's monitor is left to serialising reveals and break-glass changes. ## 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; then the parked operation must finish. Both tests fail without the fix. ## Verification `./mvnw verify` on this head (`c2fde97`): 114 tests, 0 failures, 0 Checkstyle violations, coverage checks met, BUILD SUCCESS. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
james.maes added 1 commit 2026-11-14 09:43:02 +00:00
fix: a reveal and the access-log writer no longer wait for each other
maven / build (push) Successful in 2m0s
build / maven (push) Successful in 2m0s
c2fde973a5
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>
All checks were successful
maven / build (push) Successful in 2m0s
build / maven (push) Successful in 2m0s
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-0.1:fix/reveal-access-log-deadlock-0.1
git checkout fix/reveal-access-log-deadlock-0.1
Sign in to join this conversation.