fix: atomic usage recording to prevent race conditions #53

Open

Charon wants to merge 1 commit from feat/fix-usage-race-condition into dev

Charon commented

2026-03-24 13:10:10 +00:00

Member

Fixes #42

Summary

Added checkAndRecordUsage() and checkAndRecordNamespaceUsage() methods that atomically check entitlement limits and record usage within a DB::transaction() using lockForUpdate(), preventing concurrent requests from both passing the limit check
Wrapped provisionPackage() and provisionNamespacePackage() in transactions with lockForUpdate() to prevent duplicate active base packages from concurrent provisioning
Wrapped provisionBoost() in a transaction with lockForUpdate() and added blesta_addon_id duplicate detection to prevent double-provisioning from webhook retries
Wrapped provisionNamespaceBoost() in a transaction with row-level locking
Added getLockedCurrentUsage() and getLockedNamespaceCurrentUsage() for pessimistic-locked usage queries that bypass cache

Backward Compatibility

The existing can() + recordUsage() pattern is preserved. Callers should migrate to checkAndRecordUsage() for race-safe usage tracking.

Test plan

Verify checkAndRecordUsage() correctly denies when limit is reached
Verify provisionPackage() cancels existing base before creating new one under concurrent load
Verify provisionBoost() deduplicates on blesta_addon_id
Run existing EntitlementService test suite

🤖 Generated with Claude Code

Fixes #42 ## Summary - Added `checkAndRecordUsage()` and `checkAndRecordNamespaceUsage()` methods that atomically check entitlement limits and record usage within a `DB::transaction()` using `lockForUpdate()`, preventing concurrent requests from both passing the limit check - Wrapped `provisionPackage()` and `provisionNamespacePackage()` in transactions with `lockForUpdate()` to prevent duplicate active base packages from concurrent provisioning - Wrapped `provisionBoost()` in a transaction with `lockForUpdate()` and added `blesta_addon_id` duplicate detection to prevent double-provisioning from webhook retries - Wrapped `provisionNamespaceBoost()` in a transaction with row-level locking - Added `getLockedCurrentUsage()` and `getLockedNamespaceCurrentUsage()` for pessimistic-locked usage queries that bypass cache ## Backward Compatibility The existing `can()` + `recordUsage()` pattern is preserved. Callers should migrate to `checkAndRecordUsage()` for race-safe usage tracking. ## Test plan - [ ] Verify `checkAndRecordUsage()` correctly denies when limit is reached - [ ] Verify `provisionPackage()` cancels existing base before creating new one under concurrent load - [ ] Verify `provisionBoost()` deduplicates on `blesta_addon_id` - [ ] Run existing EntitlementService test suite 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Charon added 1 commit 2026-03-24 13:10:10 +00:00

fix: atomic usage recording and provisioning to prevent race conditions ef7ed4ca77

Addresses #42 — race condition in EntitlementService usage recording.

Changes:

1. New atomic check-and-record methods:
   - checkAndRecordUsage() — workspace-scoped
   - checkAndRecordNamespaceUsage() — namespace-scoped
   These use DB::transaction() with lockForUpdate() on usage records
   to prevent concurrent requests from reading the same count and
   both passing the limit check (TOCTOU race).

2. New locked usage query methods:
   - getLockedCurrentUsage() — bypasses cache, uses SELECT FOR UPDATE
   - getLockedNamespaceCurrentUsage() — namespace equivalent
   These are used within the atomic transactions to get a consistent
   usage count under pessimistic locking.

3. provisionPackage() and provisionNamespacePackage():
   Wrapped base-package check-and-cancel in DB::transaction() with
   lockForUpdate() to prevent duplicate active base packages from
   concurrent provisioning requests.

4. provisionBoost():
   Wrapped in DB::transaction() with lockForUpdate() on existing
   boosts. Added duplicate detection for blesta_addon_id to prevent
   the same external addon from being provisioned twice.

5. provisionNamespaceBoost():
   Wrapped in DB::transaction() with lockForUpdate() on existing
   namespace boosts to serialise concurrent provisioning.

The existing can() + recordUsage() pattern is preserved for backward
compatibility. Callers should migrate to checkAndRecordUsage() for
race-safe usage tracking.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

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

git fetch -u origin feat/fix-usage-race-condition:feat/fix-usage-race-condition

git checkout feat/fix-usage-race-condition

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout dev

git merge --no-ff feat/fix-usage-race-condition

git checkout feat/fix-usage-race-condition

git rebase dev

git checkout dev

git merge --ff-only feat/fix-usage-race-condition

git checkout feat/fix-usage-race-condition

git rebase dev

git checkout dev

git merge --no-ff feat/fix-usage-race-condition

git checkout dev

git merge --squash feat/fix-usage-race-condition