Bug: two programs sharing one database can both start a scan at the same moment

Aperta
  • Qualcosa non va?
#97 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
56/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
mysql, python
Ambito
backend, database

Direzione di ricerca

Read backend/data_access/network_data/scan_store.py and DatabaseEngine.unit_of_work() to determine whether one connection can hold a MySQL named lock through commit; then inspect backend/services/scan/scan_session.py and ScanStoreLike. Implement the described atomic start flow and update FakeScanStore in tests/services/scan/test_scan_session.py. Run the named-lock and concurrency cases in tests/services/scan/test_scan_session_db.py; done means simultaneous opens on one database start exactly one scan and the README explains the single-database rule.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

backend bug help wanted High Priority

Only one scan may run at a time (CM-15). ScanSessionController.open() enforces this in two steps: it asks the database whether a scan is running (find_running()), and if none is, it saves the new scan as running (start()). The lock around those two steps, self.lock, is a Python threading.Lock, so it only stops threads inside one program. If two programs use the same database (two server processes, or .\dev scan on two machines pointed at one MySQL), both can run find_running() before either has saved its scan. Both see "nothing running", both save a running scan, and both scans run. This breaks CM-15 and leaves two running rows. find_running() then returns only the newest, so the other scan can't be seen or stopped from the status screen until recover_stuck_scans() runs. Found while testing on two machines (see "Not this bug" below).

How to reproduce

  • Point two copies of the code at the same database (the same DB_HOST, DB_PORT and DB_NAME in both .env files).
  • Start a scan from both within the same second: .\dev scan 192.168.1.0/24 --user scanner
  • Both start. SELECT scan_id, status, started_at FROM scan WHERE status = 'running'; shows two rows.
  • The window is small: a scan started a few seconds after the other is correctly refused, so the race needs near-simultaneous starts. The integration test below hits it on purpose with a barrier.

Fix: take the lock in MySQL, which every program shares

  • In backend/data_access/network_data/scan_store.py, add start_if_idle(scan) -> tuple[Scan | None, Scan | None]. It returns (new scan, None) when it started one and (None, the running scan) when it was refused. It runs, on one connection:
    1. SELECT GET_LOCK('network_scanner.scan_open', 5). Returns 1 when we hold the lock. A result of 0 (another program held it for 5 s) or NULL (error) means refuse with "Another scan is starting. Try again in a moment." and save nothing.
    2. In one unit of work: find_running(). If a scan is running, return it. Otherwise insert the new scan the same way start() does.
    3. Commit.
    4. SELECT RELEASE_LOCK('network_scanner.scan_open'), in a finally, so it is released even when step 2 fails.
  • The lock must be released after the commit. Released before it, the second program could check before the first program's row is visible and start anyway.
  • GET_LOCK belongs to one connection, so the lock, the check, the insert and the release must all use the same connection. If DatabaseEngine.unit_of_work() can't hold one connection across the commit, add a small DatabaseEngine.named_lock(name, timeout_s) context manager that does.
  • MySQL frees the lock by itself if the connection drops, so a crashed program can't block scans forever.
  • In backend/services/scan/scan_session.py, open() calls scan_store.start_if_idle(...) instead of find_running() then start(). It keeps self.lock (cheap, and it covers _retry_pending_closes() and the in-memory progress) and builds the same _still_running_message(running) refusal from the returned scan.
  • Add start_if_idle to the ScanStoreLike Protocol, and to FakeScanStore in tests/services/scan/test_scan_session.py, where it does the same check and insert under one threading.Lock.
  • Keep start() for the tests and tools that need to create a scan directly.

Why not something else

  • A UNIQUE index so the database refuses a second running row. MySQL 5.7 can do this with a generated column (running_guard = 1 when running, NULL otherwise, with a UNIQUE index). It's stronger, but it needs a schema change and a migration on every database, and it turns a refusal into a duplicate-key error we would have to translate. It's worth considering later; GET_LOCK fixes the bug without touching the schema.
  • SELECT ... FOR UPDATE on the running rows. When no scan is running there is no row to lock, so it doesn't stop the race this bug is about.

Not this bug
Two machines that each use their own MAMP database (DB_HOST=127.0.0.1, the default) can both run a scan. Each records its scan in its own database, so neither can know about the other. That's expected: one installation means one database. Testing across machines needs DB_HOST on the second machine set to the first machine's address, with MySQL there allowing remote connections.

Tests

  • Unit (test_scan_session.py): open() uses start_if_idle, and a refused start returns the running scan's message, as before.
  • Integration (test_scan_session_db.py): two threads, each with its own DatabaseEngine and controller (separate connections, like two programs), wait on a threading.Barrier(2) and call open() together. Exactly one starts; the other is refused naming the first. Afterwards there is exactly one running row. Repeat 20 times to show it isn't luck, and clean up in tearDown with recover_stuck_scans().
  • Integration: while one connection holds GET_LOCK('network_scanner.scan_open', 0), open() from another engine waits up to 5 s, then refuses with "Another scan is starting" and saves nothing.
  • Every existing test_scan_session* and test_scan_manager test still passes.

Done when

  • Two programs on one database calling open() at the same moment start exactly one scan, every time.
  • The refusal reads the same as today: "A scan started by X at HH:MM is still running."
  • The README's "Running a scan (#44)" section says one database means one running scan, however many programs or machines use it.

Milestone: M4. Related: #45, #48, CM-15.

Lingua principale
Python
Stelle
1
Fork
1
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Bearkat-Professionals/Software-Engineering-Project

Tutte le issue di Bearkat-Professionals/Software-Engineering-Project

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.