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

Open
  • Is something wrong?
#97 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
56/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
mysql, python
Domain
backend, database

Research direction

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.

Written by the indexing model from the issue text.

Description

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.

Dominant language
Python
Stars
1
Forks
1
Avg merge
8h 51m
Merged PRs (30d)
39

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Bearkat-Professionals/Software-Engineering-Project

All issues in Bearkat-Professionals/Software-Engineering-Project

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.