MEDIUM: Add file locking for concurrent JSON persistence #37

New Issue

Koko210 · 2026-02-20T14:47:47+02:00

Koko210 commented

2026-02-20 14:47:47 +02:00

Problem

Multiple modules persist state to JSON files with no file-level locking. Both the Discord bot event loop and the FastAPI API endpoints can write to the same files concurrently, risking data corruption:

Affected Files and Writers

JSON File	Written By	Also Written By
memory/servers.json	server_manager.py (bot events)	api.py (REST endpoints)
memory/dm_logs/*.json	dm_logger.py (on_message)	api.py (delete endpoints)
memory/figurine_subscribers.json	figurine_notifier.py (scheduler)	api.py (subscribe endpoint)
memory/bedtime_tracking.json	scheduled.py (scheduler)	api.py (bedtime endpoints)
memory/evil_mode_state.json	evil_mode.py (toggle)	api.py (evil toggle endpoint)
memory/bipolar_scores.json	bipolar_mode.py (argument end)	api.py (bipolar endpoints)

Race Condition Scenario

Bot event loop reads servers.json
API endpoint reads servers.json (same version)
Bot event loop writes updated servers.json
API endpoint writes its update — overwrites bot's changes (lost update)

Since these are full-file rewrites (json.dump), partial writes during crashes can also produce corrupted JSON.

Proposed Solutions (pick one)

Option A: fcntl file locking (minimal change)

import fcntl
def safe_json_write(path, data):
    with open(path, 'w') as f:
        fcntl.flock(f, fcntl.LOCK_EX)
        json.dump(data, f, indent=2)
        f.flush()
        os.fsync(f.fileno())
        fcntl.flock(f, fcntl.LOCK_UN)

Option B: Atomic write pattern

Write to a temp file, then os.rename() (atomic on Linux):
def atomic_json_write(path, data):
tmp = path + '.tmp'
with open(tmp, 'w') as f:
json.dump(data, f, indent=2)
f.flush()
os.fsync(f.fileno())
os.rename(tmp, path)

Option C: Migrate to SQLite

Replace all JSON persistence with a single SQLite database. Gets ACID transactions, no corruption risk, better query performance. Higher effort but most robust.

Impact

Risk: Low (file locking is well-understood)
Effort: Medium (create a shared utility, update all write sites)
Benefit: Eliminates data corruption risk from concurrent writes

Files Affected

bot/server_manager.py
bot/utils/dm_logger.py
bot/utils/figurine_notifier.py
bot/utils/scheduled.py
bot/utils/evil_mode.py
bot/utils/bipolar_mode.py
NEW: bot/utils/file_utils.py (shared safe write function)

## Problem Multiple modules persist state to JSON files with no file-level locking. Both the Discord bot event loop and the FastAPI API endpoints can write to the same files concurrently, risking data corruption: ### Affected Files and Writers | JSON File | Written By | Also Written By | |-----------|-----------|----------------| | memory/servers.json | server_manager.py (bot events) | api.py (REST endpoints) | | memory/dm_logs/*.json | dm_logger.py (on_message) | api.py (delete endpoints) | | memory/figurine_subscribers.json | figurine_notifier.py (scheduler) | api.py (subscribe endpoint) | | memory/bedtime_tracking.json | scheduled.py (scheduler) | api.py (bedtime endpoints) | | memory/evil_mode_state.json | evil_mode.py (toggle) | api.py (evil toggle endpoint) | | memory/bipolar_scores.json | bipolar_mode.py (argument end) | api.py (bipolar endpoints) | ### Race Condition Scenario 1. Bot event loop reads servers.json 2. API endpoint reads servers.json (same version) 3. Bot event loop writes updated servers.json 4. API endpoint writes its update — overwrites bot's changes (lost update) Since these are full-file rewrites (json.dump), partial writes during crashes can also produce corrupted JSON. ## Proposed Solutions (pick one) ### Option A: fcntl file locking (minimal change) import fcntl def safe_json_write(path, data): with open(path, 'w') as f: fcntl.flock(f, fcntl.LOCK_EX) json.dump(data, f, indent=2) f.flush() os.fsync(f.fileno()) fcntl.flock(f, fcntl.LOCK_UN) ### Option B: Atomic write pattern Write to a temp file, then os.rename() (atomic on Linux): def atomic_json_write(path, data): tmp = path + '.tmp' with open(tmp, 'w') as f: json.dump(data, f, indent=2) f.flush() os.fsync(f.fileno()) os.rename(tmp, path) ### Option C: Migrate to SQLite Replace all JSON persistence with a single SQLite database. Gets ACID transactions, no corruption risk, better query performance. Higher effort but most robust. ## Impact - Risk: Low (file locking is well-understood) - Effort: Medium (create a shared utility, update all write sites) - Benefit: Eliminates data corruption risk from concurrent writes ## Files Affected - bot/server_manager.py - bot/utils/dm_logger.py - bot/utils/figurine_notifier.py - bot/utils/scheduled.py - bot/utils/evil_mode.py - bot/utils/bipolar_mode.py - NEW: bot/utils/file_utils.py (shared safe write function)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Koko210/miku-discord#37