crud.py

@@ -178,10 +178,9 @@ async def is_available(room_id: str, check_in: str, check_out: str) -> bool:
     overlaps [check_in, check_out). This is the authoritative check; call
     it inside the same request path that writes the `held` booking.
 
-    NOTE: this read + the subsequent `held` write must be atomic per room.
+    NOTE: for production, wrap the check+hold in a transaction (or a
-    `services.request_booking` holds a per-room `asyncio.Lock` around this
+    per-room asyncio lock) so two simultaneous requests can't both pass
-    call and `create_booking` for exactly that reason — don't call this as
+    the read before either writes. See docs/event-flow.md § Concurrency.
-    the basis for a hold outside that lock. See docs/event-flow.md § Concurrency.
     """
     room = await get_room(room_id)
     if not room or room.status != RoomStatus.active:

docs/event-flow.md

@@ -120,23 +120,17 @@ the result.
 
 ## Concurrency — the check-then-hold lock
 
-`is_available()` followed by writing the `held` row is the critical section:
+`is_available()` followed by writing the `held` row is the critical section.
-two simultaneous requests for the same nights could both pass the read before
+Two simultaneous requests for the same nights could both pass the read
-either writes. Because the check counts *occupying* bookings (`held` included),
+before either writes. Required mitigation (marked `TODO(concurrency)` in
-once one request wins and writes `held`, the loser's re-check fails →
+`views_api.py`):
-`Unavailable`/`409` — so the fix only needs to make that read+write atomic.
 
-**Implemented** (`services.request_booking`): a per-room `asyncio.Lock`
+- wrap check + insert in a DB transaction, **or**
-(`_room_locks[room_id]`) wraps exactly the `is_available` → `create_booking`
+- take a per-room `asyncio.Lock` around the section.
-pair. FX conversion and invoice creation are computed outside the lock so it's
-held only for the DB critical section. Both doors (HTTP + RPC) go through
-`services.request_booking`, so the lock covers every entry point.
 
-**Scope / caveat:** an `asyncio.Lock` only serializes within one event loop.
+Because the check reads *occupying* bookings (`held` included), once one
-LNbits runs a single worker, so this is sufficient today. If it ever runs
+request wins and writes `held`, the loser's re-check fails → `409`. The DB
-multi-worker/multi-process, this must become a DB-level guard — a Postgres
+is the arbiter; the lock just makes the read-write atomic.
-exclusion constraint on the date range, or `SELECT … FOR UPDATE` on the room
-row — since separate processes don't share the lock.
 
 ## Cancellation & refunds (design intent)
 

services.py

@@ -12,8 +12,6 @@ error message — relays them to the caller verbatim; `BookingError` is a
 backend failure and surfaces as a generic error over RPC (logged server-side).
 """
 
-import asyncio
-from collections import defaultdict
 from datetime import datetime, timedelta, timezone
 
 from lnbits.core.services import create_invoice
@@ -31,13 +29,6 @@ from .models import (
     RoomStatus,
 )
 
-# Per-room lock serializing the availability read + the `held` write, so two
-# concurrent requests for the same nights can't both pass the check before
-# either commits the hold (double-booking). Keyed by room id; the dict grows
-# by distinct rooms only (bounded for a castle). Single-asyncio-loop scope —
-# see the note in request_booking on the multi-worker caveat.
-_room_locks: dict[str, asyncio.Lock] = defaultdict(asyncio.Lock)
-
 
 class NotFound(ValueError):
     """Referenced entity does not exist (-> HTTP 404)."""
@@ -99,8 +90,10 @@ async def request_booking(data: BookingRequestData) -> BookingQuote:
     if data.num_guests > room.max_guests:
         raise ValueError(f"Max {room.max_guests} guests")
 
-    # Compute the canonical amount up front (FX call) so the lock below wraps
+    # TODO(#4): wrap is_available + create_booking in a per-room lock / txn.
-    # only the DB check + insert, never the slow network work.
+    if not await crud.is_available(data.room_id, data.check_in, data.check_out):
+        raise Unavailable("Those dates are no longer available")
+
     settings = await crud.get_or_create_settings()
     price_fiat = round(room.price_amount * nights, 2)
     amount_sat = await to_sats(price_fiat, room.price_currency)  # canonical
@@ -123,21 +116,6 @@ async def request_booking(data: BookingRequestData) -> BookingQuote:
         expires_at=datetime.now(timezone.utc)
         + timedelta(minutes=settings.default_hold_minutes),
     )
-
-    # Atomic check-then-hold. The `held` row is itself the lock on the dates
-    # (is_available counts held as occupying), so serializing the read+insert
-    # per room means the first request to commit wins and every later one sees
-    # it and gets Unavailable. FX + invoice creation stay outside the lock.
-    #
-    # Scope: a single asyncio loop. LNbits runs one worker, so an asyncio.Lock
-    # is sufficient; if it ever runs multi-worker/multi-process this must move
-    # to a DB-level guard (Postgres exclusion constraint or SELECT ... FOR
-    # UPDATE) — noted in issue #4 / event-flow.md.
-    async with _room_locks[room.id]:
-        if not await crud.is_available(
-            data.room_id, data.check_in, data.check_out
-        ):
-            raise Unavailable("Those dates are no longer available")
     await crud.create_booking(booking)
 
     # Sats-denominated (deposit_sat locked at quote time) so FX drift before