Add concurrency protection for Fava/Beancount ledger writes #5

Closed

padreug wants to merge 0 commits from fix/concurrency-protection into main

pull from: fix/concurrency-protection

merge into: aiolabs:main

aiolabs:main

aiolabs:fix/authorization-security-refactor

Conversation 1 Commits - Files changed -

padreug commented 2026-01-06 22:57:27 +00:00

Owner

Copy link

Summary

This PR addresses critical race conditions when multiple requests try to write to the Fava/Beancount ledger file simultaneously. Without these protections, concurrent writes can cause data loss, duplicate entries, or file corruption.

Fixes #4

---

Fix Implemented

The following concurrency protections have been implemented:

1. Global Write Lock in FavaClient

Added asyncio.Lock() to serialize all write operations to the Fava/Beancount ledger:

class FavaClient:
    def __init__(self, ...):
        self._write_lock = asyncio.Lock()
        self._user_locks: Dict[str, asyncio.Lock] = {}

2. Protected Write Methods

The following methods now acquire the global write lock before modifying the ledger:

• add_entry() - Wrapped with async with self._write_lock:
• add_account() - Wrapped with lock + retry logic for checksum conflicts
• update_entry_source() - Wrapped with lock
• delete_entry() - Wrapped with lock

3. Retry Logic with Exponential Backoff

add_account() now implements optimistic concurrency control:

• Retries up to 3 times on checksum conflict (HTTP 409/412)
• Exponential backoff: 0.1s, 0.2s, 0.4s between retries
• Re-checks if account was created by concurrent request before retrying
• Raises ChecksumConflictError if all retries fail

4. Idempotent Entry Creation

New add_entry_idempotent() method:

result = await fava.add_entry_idempotent(entry, idempotency_key="ln-abc123")
if result.get("existing"):
    # Entry already existed, no duplicate created

• Checks if entry with idempotency key (as Beancount link) already exists
• Prevents duplicate entries even when same operation is retried
• Returns existing entry data if found

5. Per-User Locking

Added get_user_lock(user_id) method for user-specific operations:

user_lock = fava.get_user_lock(user_id)
async with user_lock:
    # User-specific operations serialized

6. Protected Invoice Payment Processing

Updated on_invoice_paid() in tasks.py:

• Uses per-user lock to serialize payment processing per user
• Uses add_entry_idempotent() with payment hash as idempotency key
• Prevents duplicate settlement entries even if same payment processed twice

---

Files Changed

• fava_client.py - Added locks, retry logic, idempotent method
• tasks.py - Updated on_invoice_paid() to use new protections

What This Fixes

Scenario	Before	After
Two users submit expenses simultaneously	Data loss possible	Serialized, both recorded
Same invoice processed twice	Duplicate entries	Single entry (idempotent)
Concurrent account creation	409 error	Retry succeeds
User A and B simultaneous requests	Can corrupt each other	Per-user isolation

Test Plan

• Verify concurrent expense submissions are serialized (no data loss)
• Verify same payment hash processed twice creates only one entry
• Verify account creation conflicts retry and succeed
• Verify users don't block each other (per-user locks)
• Run existing tests to ensure no regressions

Limitations

• Global write lock serializes ALL writes (potential bottleneck at scale)
• For horizontal scaling, distributed locking (Redis) would be needed
• Per-user locks are in-memory only (don't persist across restarts)

🤖 Generated with Claude Code

## Summary This PR addresses **critical race conditions** when multiple requests try to write to the Fava/Beancount ledger file simultaneously. Without these protections, concurrent writes can cause data loss, duplicate entries, or file corruption. Fixes #4 --- ## Fix Implemented The following concurrency protections have been implemented: ### 1. Global Write Lock in FavaClient Added `asyncio.Lock()` to serialize all write operations to the Fava/Beancount ledger: ```python class FavaClient: def __init__(self, ...): self._write_lock = asyncio.Lock() self._user_locks: Dict[str, asyncio.Lock] = {} ``` ### 2. Protected Write Methods The following methods now acquire the global write lock before modifying the ledger: - `add_entry()` - Wrapped with `async with self._write_lock:` - `add_account()` - Wrapped with lock + retry logic for checksum conflicts - `update_entry_source()` - Wrapped with lock - `delete_entry()` - Wrapped with lock ### 3. Retry Logic with Exponential Backoff `add_account()` now implements optimistic concurrency control: - Retries up to 3 times on checksum conflict (HTTP 409/412) - Exponential backoff: 0.1s, 0.2s, 0.4s between retries - Re-checks if account was created by concurrent request before retrying - Raises `ChecksumConflictError` if all retries fail ### 4. Idempotent Entry Creation New `add_entry_idempotent()` method: ```python result = await fava.add_entry_idempotent(entry, idempotency_key="ln-abc123") if result.get("existing"): # Entry already existed, no duplicate created ``` - Checks if entry with idempotency key (as Beancount link) already exists - Prevents duplicate entries even when same operation is retried - Returns existing entry data if found ### 5. Per-User Locking Added `get_user_lock(user_id)` method for user-specific operations: ```python user_lock = fava.get_user_lock(user_id) async with user_lock: # User-specific operations serialized ``` ### 6. Protected Invoice Payment Processing Updated `on_invoice_paid()` in tasks.py: - Uses per-user lock to serialize payment processing per user - Uses `add_entry_idempotent()` with payment hash as idempotency key - Prevents duplicate settlement entries even if same payment processed twice --- ## Files Changed - `fava_client.py` - Added locks, retry logic, idempotent method - `tasks.py` - Updated `on_invoice_paid()` to use new protections ## What This Fixes | Scenario | Before | After | |----------|--------|-------| | Two users submit expenses simultaneously | Data loss possible | Serialized, both recorded | | Same invoice processed twice | Duplicate entries | Single entry (idempotent) | | Concurrent account creation | 409 error | Retry succeeds | | User A and B simultaneous requests | Can corrupt each other | Per-user isolation | ## Test Plan - [ ] Verify concurrent expense submissions are serialized (no data loss) - [ ] Verify same payment hash processed twice creates only one entry - [ ] Verify account creation conflicts retry and succeed - [ ] Verify users don't block each other (per-user locks) - [ ] Run existing tests to ensure no regressions ## Limitations - Global write lock serializes ALL writes (potential bottleneck at scale) - For horizontal scaling, distributed locking (Redis) would be needed - Per-user locks are in-memory only (don't persist across restarts) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 1 commit 2026-01-06 22:57:28 +00:00

Add concurrency protection for Fava/Beancount ledger writes ... b5c36504fb

This commit addresses critical race conditions when multiple requests
try to write to the ledger file simultaneously.

Changes:
- Add global asyncio.Lock to FavaClient to serialize all write operations
- Add per-user locks for finer-grained concurrency control
- Wrap add_entry(), update_entry_source(), delete_entry() with write lock
- Add retry logic with exponential backoff to add_account() for checksum conflicts
- Add new add_entry_idempotent() method to prevent duplicate entries
- Add ChecksumConflictError exception for conflict handling
- Update on_invoice_paid() to use per-user locking and idempotent entry creation

Fixes #4

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

padreug commented 2026-05-07 06:06:19 +00:00

Author

Owner

Copy link

Closing — this work landed directly on main outside the PR UI.

The PR head commit b5c3650 ("Add concurrency protection for Fava/Beancount ledger writes") is reachable from main. Verified locally:

• fava_client.py contains _write_lock, _user_locks, get_user_lock(), add_entry_idempotent(), and ChecksumConflictError
• tasks.py uses per-user locks and idempotent entry creation in on_invoice_paid()

$ git branch --contains b5c3650
* main

No further action needed.

Closing — this work landed directly on `main` outside the PR UI. The PR head commit `b5c3650` ("Add concurrency protection for Fava/Beancount ledger writes") is reachable from `main`. Verified locally: - `fava_client.py` contains `_write_lock`, `_user_locks`, `get_user_lock()`, `add_entry_idempotent()`, and `ChecksumConflictError` - `tasks.py` uses per-user locks and idempotent entry creation in `on_invoice_paid()` ``` $ git branch --contains b5c3650 * main ``` No further action needed.

padreug closed this pull request 2026-05-07 06:12:12 +00:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

aiolabs/libra!5

No description provided.