Skip to content

fix: run blocking verify calls in threadpool#296

Open
asnaassalam wants to merge 2 commits into
ionfwsrijan:mainfrom
asnaassalam:fix/verify-endpoint-blocking
Open

fix: run blocking verify calls in threadpool#296
asnaassalam wants to merge 2 commits into
ionfwsrijan:mainfrom
asnaassalam:fix/verify-endpoint-blocking

Conversation

@asnaassalam

Copy link
Copy Markdown

Before opening: make sure there is an issue tracking this work, and link it below. PRs without a linked issue may be closed without review.

Linked issue

Closes #224

What this PR does

The /verify endpoint called verify_repo() and _scan_repo_dir() directly inside an async route. Both are blocking (synchronous) calls, so they occupied the single event loop for their full duration freezing all other requests until verification finished. This PR wraps both calls in run_in_threadpool so they run in a separate thread instead of blocking the main event loop.

Type of change

  • Bug fix
  • New feature
  • ML model / training pipeline
  • Refactor (no behaviour change)
  • Documentation
  • Tests only

ML tier (if applicable)

  • Tier 1 — Triage
  • Tier 2 — Predictive
  • Tier 3 — Autonomous
  • Not ML-related

Stack affected

  • Backend
  • Frontend
  • Both

Changes

Backend

  • Wrapped verify_repo(repo_dir) call in run_in_threadpool.
  • Wrapped _scan_repo_dir(...) call in run_in_threadpool using functools.partial, following the same pattern already used elsewhere in main.py (e.g. the scan endpoint).
  • No change to the internal logic of either function, only how they're invoked.

Frontend

  • No frontend changes.

New dependencies

  • None. run_in_threadpool is part of FastAPI, already a dependency, no new package added.

Database / schema changes

  • None

Testing

How did you test this?

  • Temporarily added time.sleep(10) inside verify_repo to simulate slow verification.
  • Ran the server locally and called /verify via /docs.
  • While /verify was still running (~10-12 seconds), fired a second unrelated request (/docs) in a separate tab and confirmed it responded instantly instead of waiting, proving the event loop stayed free.
  • Removed the temporary sleep before pushing.
  • Re-tested /verify normally to confirm it still returns a correct 200 OK response.

Checklist

  • Tested locally end-to-end (upload ZIP or GitHub URL → scan → findings returned correctly)
  • New ML model falls back gracefully when model file is absent
  • No new console.error or unhandled Python exceptions introduced
  • Added or updated tests where applicable
  • requirements.txt / package.json updated if new dependencies added
  • New model files (.pkl, .pt, etc.) are gitignored, not committed

Anything reviewers should focus on

Followed the same run_in_threadpool + functools.partial pattern already used elsewhere in main.py for consistency. Please confirm this doesn't introduce any thread-safety issues in verify_repo or _scan_repo_dir.

Screenshots (if UI changed)

N/A (Backend-only changes)

Moves verify_repo and _scan_repo_dir calls in the /verify endpoint
to run_in_threadpool so they no longer block the event loop while
processing. Fixes ionfwsrijan#224.
Copilot AI review requested due to automatic review settings July 8, 2026 10:49
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

🎉 Thank you @asnaassalam for submitting a Pull Request!

We're excited to review your contribution.

Before Review

✅ Ensure all CI checks pass
✅ Complete the PR template
✅ Link the related issue

Want faster reviews and contributor support?

Join our Discord community:

🔗 https://discord.gg/FcXuyw2Rs

Maintainers and mentors are active there and can help resolve blockers quickly.

Happy Contributing! 🚀

@github-actions github-actions Bot added backend Backend issues bug Something isn't working frontend Frontend issues SSoC26 labels Jul 8, 2026
@github-actions github-actions Bot requested a review from arpit2006 July 8, 2026 10:50
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR template check passed!

@arpit2006 this PR is ready for your review. 🚀

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a scalability/performance bug in the backend by preventing the async /verify FastAPI route from executing long-running synchronous verification and scanning work on the main event loop thread.

Changes:

  • Offloaded verify_repo(repo_dir) from the /verify async route into run_in_threadpool.
  • Offloaded _scan_repo_dir(...) from the /verify async route into run_in_threadpool (via functools.partial), matching existing patterns in main.py.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@arpit2006

Copy link
Copy Markdown
Collaborator

@asnaassalam , Please Resolve CI issue!

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR template check passed!

@arpit2006 this PR is ready for your review. 🚀

@arpit2006

Copy link
Copy Markdown
Collaborator

@asnaassalam , CI issue

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

Labels

assigned backend Backend issues bug Something isn't working changes-required frontend Frontend issues Medium Medium difficulty needs-work Work needed PR-review SSoC26 Template-passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Performance] Blocking Operations in Async /verify Endpoint Can Stall FastAPI Event Loop

3 participants