fix(usage): take the withdrawal baseline before the lease, not after it
Third and last window in the same race, and again in my own fix. locked() claims the lease and reads the row in two separate statements. Reading the cancellation counter from inside that callback meant a /disable completing in the gap was adopted as this activation's own baseline and silently absorbed — the counter matched, the claim succeeded, and registration went ahead after the operator had withdrawn. The baseline is now read before the lease is taken, which inverts it: every increment from that point on is later than the value the claim tests for, so the claim fails and the withdrawal wins. An increment from before the read is a withdrawal the operator already completed, and a deliberate opt-in afterwards should not be vetoed by it. The test for this passed against the bug on its first two attempts. It stubbed the state read to increment the counter AFTER reading the row, so both the broken and the fixed version saw the old value and behaved identically. The withdrawal has to land before the read returns for the row to carry it — which is the whole point of the window. It now fails without the fix. Refs #1110
This commit is contained in:
@@ -214,20 +214,22 @@ class UsageService {
|
||||
async enable(consent) {
|
||||
if (consent !== 'usage-consent.v1')
|
||||
throw new ValidationError('Explicit usage consent is required');
|
||||
// Read BEFORE the lease, deliberately. locked() claims the lease and then
|
||||
// reads the row in a second statement; a /disable completing between
|
||||
// those two would be adopted as this activation's own baseline and
|
||||
// silently absorbed. Taking the baseline first inverts that: every
|
||||
// increment from this point on — including one in that gap — is later
|
||||
// than the value the claim tests for, so the claim fails and the
|
||||
// withdrawal wins. An increment from BEFORE this read is a withdrawal the
|
||||
// operator already completed, and a deliberate opt-in afterwards should
|
||||
// not be vetoed by it.
|
||||
const cancelSeq = Number((await this.state())?.cancel_seq || 0);
|
||||
await this.locked(async (state) => {
|
||||
if (state.status !== 'disabled')
|
||||
throw new ConflictError(
|
||||
'Finish the current participation before rejoining'
|
||||
);
|
||||
this.collectorUrl();
|
||||
// The withdrawal counter as it stood when this activation began. A
|
||||
// /disable from an earlier participation is already reflected here and
|
||||
// must not veto a deliberate opt-in; anything that increments it from
|
||||
// now on is aimed at THIS activation. Recorded rather than cleared,
|
||||
// because a clearing write of its own had the very race it was meant to
|
||||
// close — a /disable landing between the lease and the clear was erased.
|
||||
const cancelSeq = Number(state.cancel_seq || 0);
|
||||
|
||||
const identity = generateIdentity();
|
||||
const pending = makePacket(identity, 'register', 0, {
|
||||
consent_version: consent
|
||||
|
||||
Reference in New Issue
Block a user