Skip to content

hosted_rooms: add cross-process lock to prevent state.db corruption on concurrent gateway startup - #102176

Open
Sahilvishnaliya wants to merge 1 commit into
NousResearch:mainfrom
Sahilvishnaliya:fix/102120-hosted-room-worker-lock
Open

Sahilvishnaliya wants to merge 1 commit into
NousResearch:mainfrom
Sahilvishnaliya:fix/102120-hosted-room-worker-lock

Conversation

@Sahilvishnaliya

Copy link
Copy Markdown
Contributor

Fixes #102120

The hosted_room_worker starts on every profile gateway restart and calls
prune_disbanded_rooms() on the shared state.db. When multiple profiles
restart simultaneously (e.g. during 'hermes update' fleet restart), all
gateways hit the same SQLite database concurrently, causing corruption
('file is not a database').

This fix adds a cross-process advisory lock (fcntl on Unix, msvcrt on
Windows) that serializes access to the shared database during pruning.
The lock file is created alongside the database with a .hosted_rooms.lock
suffix.

Also adds a graceful fallback when neither locking mechanism is available
(no-op lock that at least doesn't crash).


Testing: All 44 hosted_rooms tests pass.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 3, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: #102153 addresses the reported concurrent first-open/WAL setup mechanism for #102120. This PR locks pruning instead; reviewers should choose whether that separate scope is needed.

@Sahilvishnaliya

Copy link
Copy Markdown
Contributor Author

🔴 CI failing — needs a fix before merge.

Failing check:

  • Python lints / ruff + ty diff — failure

Other jobs (tests, other lints) are passing. Please check the ruff + ty diff job log — likely a ruff lint violation or ty type-check regression introduced in this diff — and push a fix.

@Sahilvishnaliya

Copy link
Copy Markdown
Contributor Author

CI Python lints / ruff + ty diff cause confirmed in the lock prelude of gateway/hosted_rooms.py.

  1. Unused import os (F401) — the new lock code never uses os.
  2. msvcrt possibly unbound (ty) — it is only assigned inside except ImportError for fcntl, but later referenced as elif msvcrt:.
  3. Dead _is_lock_contention_errno — defined, never called; that also makes import errno unused once the helper is removed.

Fix the top of the file to:

from __future__ import annotations

import contextlib
import hashlib
import json
import re
import sqlite3
import time
from contextlib import contextmanager
from pathlib import Path
from typing import Any, Iterator, Mapping, NoReturn

# Cross-process file locking: fcntl on Unix, msvcrt on Windows.
fcntl = None
msvcrt = None
try:
    import fcntl
except ImportError:
    try:
        import msvcrt
    except ImportError:
        pass

And delete _is_lock_contention_errno entirely (unused).

Do not nest import msvcrt only under the fcntl ImportError without first setting msvcrt = None — that is the ty unbound-name failure on Unix.

…n concurrent gateway startup

Fixes NousResearch#102120

The hosted_room_worker starts on every profile gateway restart and calls
prune_disbanded_rooms() on the shared state.db. When multiple profiles
restart simultaneously (e.g. during 'hermes update' fleet restart), all
gateways hit the same SQLite database concurrently, causing corruption
('file is not a database').

This fix adds a cross-process advisory lock (fcntl on Unix, msvcrt on
Windows) that serializes access to the shared database during pruning.
The lock file is created alongside the database with a .hosted_rooms.lock
suffix.

Also adds a graceful fallback when neither locking mechanism is available
(no-op lock that at least doesn't crash).
@Sahilvishnaliya
Sahilvishnaliya force-pushed the fix/102120-hosted-room-worker-lock branch from 11d6a58 to 9b88865 Compare September 17, 2026 12:34

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hosted_room_worker corrupts shared state.db on simultaneous multi-profile gateway restart (e.g. via hermes update)

2 participants