Skip to content

fix(core): resolve throttling race condition and optimize memory cache expiration - #319

Open
AtaCanYmc wants to merge 6 commits into
yanyongyu:masterfrom
AtaCanYmc:master
Open

fix(core): resolve throttling race condition and optimize memory cache expiration#319
AtaCanYmc wants to merge 6 commits into
yanyongyu:masterfrom
AtaCanYmc:master

Conversation

@AtaCanYmc

Copy link
Copy Markdown

Description

This PR addresses critical concurrency, context management, and performance issues:

  1. Thread-Safety in LocalThrottler (githubkit/throttling.py):

    • Added threading.Lock to guard lazy initialization of _semaphore and _async_semaphore.
    • Resolves a race condition where concurrent threads could instantiate duplicate semaphores and bypass the max_concurrency limit.
  2. Harden Context Manager Teardown in GitHubCore (githubkit/core.py):

    • Updated __exit__ and __aexit__ to safely check for non-None client instances before calling .close() / .aclose().
    • Prevents potential AttributeError crashes during exception teardown.
  3. $O(1)$ Passive Cache Expiration in MemCache (githubkit/cache/mem_cache.py):

    • Replaced full $O(N)$ dictionary scans on every read/write with passive $O(1)$ item expiration during get() lookups.
  4. Unit Test Coverage (tests/test_unit_test/test_unit_test.py):

    • Added unit test coverage validating thread-safety under multi-threaded execution, passive cache expiration, and context manager cleanup.

Verification

Ran the unit test suite:

uv run pytest tests/test_unit_test

AtaCanYmc and others added 6 commits August 4, 2026 23:15
…e expiration

- Add thread-safe lock to LocalThrottler to prevent concurrent semaphore re-instantiation
- Harden GitHubCore context manager exit against None client calls
- Refactor MemCache to passive O(1) expiration on lookup/set instead of O(N) linear sweep
- Add unit tests in tests/test_unit_test/test_unit_test.py
fix(core): resolve throttling race condition and optimize memory cache expiration
…e expiration

- Add thread-safe lock to LocalThrottler to prevent concurrent semaphore re-instantiation
- Harden GitHubCore context manager exit against None client calls
- Refactor MemCache to passive O(1) expiration on lookup/set instead of O(N) linear sweep
- Add unit tests in tests/test_unit_test/test_unit_test.py
…to fix/remediate-race-condition

# Conflicts:
#	tests/test_unit_test/test_unit_test.py
@yanyongyu yanyongyu added the bug Something isn't working label Aug 5, 2026
@yanyongyu

Copy link
Copy Markdown
Owner

tests/test_unit_test/test_unit_test.py file is used to test example in the docs. You can create new files like tests/test_throttling.py to add new tests.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants