Skip to content

asyncio.Event used across thread boundaries in cancellation mechanism - causes undefined behavior #258

Description

@ionfwsrijan

Description

The scan cancellation mechanism uses an asyncio.Event object but passes it across thread boundaries, which is undefined behavior in Python's asyncio framework.

Root Cause

In backend/app/main.py, the _scan_repo_dir function (line ~191) accepts a cancel_event: asyncio.Event = None parameter:

def _scan_repo_dir(
    repo_dir: Path,
    progress_cb=None,
    job_dir: Path = None,
    cancel_event: asyncio.Event = None,
    raw_dir_name: str = "raw",
):

This function is called via run_in_threadpool() (FastAPI's thread pool executor) at line ~479:

_, _, _, _, findings = await run_in_threadpool(
    _scan_repo_dir, repo_dir, ... , cancel_event=cancel_event
)

The cancel_event.is_set() checks inside _scan_repo_dir (lines ~195, 203, 211, 219) are then executed inside a thread pool worker, not on the asyncio event loop thread.

asyncio.Event is not thread-safe. Its is_set() and set() methods are designed to be called from a single-threaded async context. Calling them from a thread pool worker constitutes a data race — undefined behavior that can manifest as:

  1. Missed cancellation signals (thread reads a stale cached value)
  2. Spurious cancellation detections (memory visibility issues across threads)
  3. Interpreter crashes in CPython's internal data structures when set() and is_set() race

This affects every scan operation: single scans, org batch scans, and verification scans.

Code Paths Affected

Path File:Line Impact
_scan_repo_dir main.py:191 Cancellation checks between scanner invocations (4 check points)
_run_repo_scan_task main.py:1324,1350,1354 Checks cancel_event before and after download/unzip
download_to_path main.py:286-369 Receives cancel_event but never checks it in the chunk loop
abort_org_scan main.py:1609 Sets the cancel_event via cancel_event.set() from the event loop

The critical problem: when download_to_path is downloading a 500MB ZIP from GitHub (30-120 seconds), it never checks cancel_event inside its chunk reading loop. Even if it did, the asyncio.Event.is_set() call would be from a thread, which is unsafe.

Proposed Fix

Replace asyncio.Event with threading.Event for cross-boundary cancellation signals. Additionally, add cancellation checks inside the download_to_path chunk loop.

Changes Required

  1. backend/app/main.py: Change all cancel_event parameters from asyncio.Event to threading.Event. Create the event at the call site using threading.Event() instead of asyncio.Event().

  2. backend/app/main.py ~line 328: Add cancel_event.is_set() check inside the chunk download loop in download_to_path:

async with client.stream("GET", current_url) as r:
    try:
        with open(dest_path, "wb") as f:
            async for chunk in r.aiter_bytes(chunk_size=chunk_size):
                if cancel_event and cancel_event.is_set():
                    raise asyncio.CancelledError("Download cancelled by user")
                bytes_received += len(chunk)
                if bytes_received > MAX_UPLOAD_SIZE:
                    raise HTTPException(status_code=413, detail="Upload too large")
                f.write(chunk)
    except Exception:
        if dest_path.exists():
            dest_path.unlink()
        raise
  1. backend/app/main.py: In _run_org_batch, use asyncio.wait(tasks, return_when=FIRST_COMPLETED) instead of asyncio.gather(*tasks) to allow immediate response when cancellation is requested.

  2. backend/app/main.py: Add a long-running task that monitors the threading.Event and cancels all remaining tasks when set.

  3. backend/app/scanners/*.py: Pass cancel_event to scanner functions and add periodic checks during long-running subprocess calls.

Metadata

Metadata

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions