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.
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)
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>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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 inpom.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.revealandBreakGlassService.start,endandexpirehold the runtime's monitor for their whole operation.classificationRows()andactiveRetentionRules()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: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 verifyon this head (c2fde97): 114 tests, 0 failures, 0 Checkstyle violations, coverage checks met, BUILD SUCCESS.🤖 Generated with Claude Code
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>