fix: make check-then-hold atomic against concurrent bookings (#4) #9

Merged

padreug merged 1 commit from feat/atomic-hold into main

2026-07-19 15:09:28 +00:00

padreug commented

2026-07-19 15:09:00 +00:00

Owner

Closes the code portion of #4.

The bug

is_available() → create_booking() is a critical section. Two simultaneous requests for the same nights could both pass the availability read before either wrote its held row → double-booking.

The fix

A per-room asyncio.Lock (_room_locks[room_id]) in services.request_booking wraps exactly the is_available → create_booking pair. Because a held row itself blocks the dates (is_available counts held as occupying), the first request to commit inside the lock wins; every later one re-checks, sees the hold, and gets Unavailable/409.

FX conversion + invoice creation are computed outside the lock — it's held only for the DB check+insert, not slow network work.
Both the HTTP door (views_api) and the RPC door (transport_rpcs) funnel through services.request_booking, so one lock covers every entry point — a payoff of the earlier services.py extraction.

Scope / caveat (documented)

An asyncio.Lock only serializes within one event loop. LNbits runs a single worker, so this is sufficient today. If it ever runs multi-worker/multi-process, this must become a DB-level guard (Postgres exclusion constraint on the date range, or SELECT … FOR UPDATE on the room row). Noted in event-flow.md and the crud.is_available docstring.

Test

No automated test yet — chatelet has no test harness, and a real concurrency test needs the LNbits pytest fixtures (DB + FakeWallet). Manual: fire two booking_requests for identical dates at once → exactly one held, one 409. Worth standing up a tests/ harness as a follow-up so this gets a regression test.

Why PR

Money-handling extension → PR category. Handing off merge to you via the Forgejo UI.

🤖 Generated with Claude Code

Closes the code portion of #4. ## The bug `is_available()` → `create_booking()` is a critical section. Two simultaneous requests for the same nights could both pass the availability read before either wrote its `held` row → **double-booking**. ## The fix A per-room `asyncio.Lock` (`_room_locks[room_id]`) in `services.request_booking` wraps exactly the `is_available` → `create_booking` pair. Because a `held` row itself blocks the dates (`is_available` counts `held` as occupying), the first request to commit inside the lock wins; every later one re-checks, sees the hold, and gets `Unavailable`/`409`. - FX conversion + invoice creation are computed **outside** the lock — it's held only for the DB check+insert, not slow network work. - Both the HTTP door (`views_api`) and the RPC door (`transport_rpcs`) funnel through `services.request_booking`, so **one lock covers every entry point** — a payoff of the earlier services.py extraction. ## Scope / caveat (documented) An `asyncio.Lock` only serializes within one event loop. LNbits runs a single worker, so this is sufficient today. If it ever runs multi-worker/multi-process, this must become a DB-level guard (Postgres exclusion constraint on the date range, or `SELECT … FOR UPDATE` on the room row). Noted in `event-flow.md` and the `crud.is_available` docstring. ## Test No automated test yet — chatelet has no test harness, and a real concurrency test needs the LNbits pytest fixtures (DB + FakeWallet). Manual: fire two `booking_request`s for identical dates at once → exactly one `held`, one `409`. Worth standing up a `tests/` harness as a follow-up so this gets a regression test. ## Why PR Money-handling extension → PR category. Handing off merge to you via the Forgejo UI. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 1 commit

2026-07-19 15:09:00 +00:00

fix: make check-then-hold atomic against concurrent bookings (#4 ) 9d87c129ad

Two simultaneous requests for the same nights could both pass is_available()
before either wrote its held row, double-booking the dates. Wrap the
is_available -> create_booking pair in a per-room asyncio.Lock
(_room_locks[room_id]) in services.request_booking, which both the HTTP and
RPC doors funnel through. FX + invoice creation stay outside the lock, so it
covers only the DB critical section.

Single-loop scope (LNbits runs one worker); documented the multi-worker
caveat (needs a DB-level guard) in event-flow.md and the crud.is_available
note.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019VUQCfdqiLSsFS2jcGnaFD

padreug merged commit 27863db444 into main

2026-07-19 15:09:28 +00:00

padreug deleted branch feat/atomic-hold

2026-07-19 15:09:29 +00:00

padreug referenced this pull request from a commit

2026-07-19 15:09:30 +00:00

Merge pull request 'fix: make check-then-hold atomic against concurrent bookings (#4)' (#9) from feat/atomic-hold into main

padreug referenced this pull request

2026-07-19 22:20:25 +00:00

Make check-then-hold atomic (double-booking race) #4