Skip to content

Thread-safe core mlc operations: rm cache, mark-tmp, repos.json#267

Draft
arjunsuresh with Copilot wants to merge 3 commits into
mainfrom
copilot/dev-make-core-mlc-operations-thread-safe
Draft

Thread-safe core mlc operations: rm cache, mark-tmp, repos.json#267
arjunsuresh with Copilot wants to merge 3 commits into
mainfrom
copilot/dev-make-core-mlc-operations-thread-safe

Conversation

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Concurrent mlc operations (rm cache, mark-tmp cache, pull repo, rm repo) share mutable state without synchronization, causing data corruption under parallel execution. This extends the file-locking pattern already used in index.py to the remaining unprotected write paths.

Changes

mlc/repo_action.py

  • Added FileLock around repos.json read-modify-write in both register_repo() and unregister_repo() — eliminates the TOCTOU race where two concurrent calls read the same list, each append/remove, and the last writer wins
  • Extracted _repos_lock_file(path) helper to centralize .lock path derivation

mlc/action.py

  • Wrapped shutil.rmtree in rm() with try/except FileNotFoundError — handles the window between os.path.exists() and actual deletion when a concurrent process removes the item first
  • Merged the duplicate try/except from the force vs. interactive branches into one

mlc/cache_action.py

  • Added per-item FileLock in mark_tmp() around the meta file read-tag-write cycle — prevents concurrent calls from writing duplicate tmp tags

tests/test_thread_safety.py (new)

  • ConcurrentRmCacheTest: two threads delete the same cache item simultaneously; asserts no exception raised
  • ConcurrentMarkTmpTest: five threads call mark-tmp on the same item; asserts exactly one tmp tag in result
  • ConcurrentReposJsonTest: ten threads concurrently call unregister_repo; asserts repos.json remains valid and all entries are removed

✅ PR Checklist

  • Target branch is dev

📌 Note: PRs must be raised against dev. Do not commit directly to main.

✅ Testing & CI

  • Have tested the changes in my local environment, else have properly conveyed in the PR description
  • The change includes a GitHub Action to test the script(if it is possible to be added).
  • No existing GitHub Actions are failing because of this change.

📚 Documentation

  • README or help docs are updated for new features or changes.
  • CLI help messages are meaningful and complete.

📁 File Hygiene & Output Handling

  • No unintended files (e.g., logs, cache, temp files, pycache, output folders) are committed.

🛡️ Safety & Security

  • No secrets or credentials are committed.
  • Paths, shell commands, and environment handling are safe and portable.

🙌 Contribution Hygiene

  • PR title and description are concise and clearly state the purpose of the change.
  • All reviewer feedback has been addressed.

Copilot AI changed the title [WIP] Make core mlc script operations thread safe Thread-safe core mlc operations: rm cache, mark-tmp, repos.json Jul 19, 2026
Copilot AI requested a review from arjunsuresh July 19, 2026 12:29
@github-actions

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@arjunsuresh
arjunsuresh requested a review from anandhu-eng July 20, 2026 10:25
@github-actions

Copy link
Copy Markdown

🤖 AI PR Review Summary\n\nThis PR introduces file locking to critical sections that read-modify-write shared resources such as cache metadata and repos.json to ensure thread safety and prevent race conditions. It adds FileLock usage in CacheAction.mark_tmp, RepoAction methods that modify repos.json, and improves error handling in Action.rm. Additionally, it includes comprehensive multithreaded unit tests to verify thread safety of cache removal, marking tmp tags, and repo registration/unregistration. The main risk is potential deadlocks or lock contention if locks are not managed properly, but the design correctly scopes locks with timeouts and minimal critical sections.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Making the core mlc script operations thread safe

2 participants