From 0d2d3c127bc49a854b1660fa49e8a9bf0cea3cd4 Mon Sep 17 00:00:00 2001 From: AtaCanYmc Date: Tue, 4 Aug 2026 23:15:59 +0300 Subject: [PATCH 1/3] fix(core): resolve throttling race condition and optimize memory cache 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 --- githubkit/cache/mem_cache.py | 10 +++-- githubkit/core.py | 14 +++++-- githubkit/throttling.py | 9 ++++- tests/test_unit_test/test_unit_test.py | 52 +++++++++++++++++++++++++- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/githubkit/cache/mem_cache.py b/githubkit/cache/mem_cache.py index 834378b6b1..38dfaad87f 100644 --- a/githubkit/cache/mem_cache.py +++ b/githubkit/cache/mem_cache.py @@ -27,8 +27,13 @@ def expire(self): @override def get(self, key: str) -> str | None: - self.expire() - return (item := self._cache.get(key, None)) and item.value + item = self._cache.get(key) + if item is None: + return None + if item.expire_at is not None and item.expire_at < datetime.now(timezone.utc): + self._cache.pop(key, None) + return None + return item.value @override async def aget(self, key: str) -> str | None: @@ -36,7 +41,6 @@ async def aget(self, key: str) -> str | None: @override def set(self, key: str, value: str, ex: timedelta) -> None: - self.expire() self._cache[key] = _Item(value, datetime.now(timezone.utc) + ex) @override diff --git a/githubkit/core.py b/githubkit/core.py index 2b5faff162..108f05625f 100644 --- a/githubkit/core.py +++ b/githubkit/core.py @@ -222,8 +222,11 @@ def __exit__( exc_value: BaseException | None = None, traceback: TracebackType | None = None, ): - cast(httpx.Client, self.__sync_client.get()).close() - self.__sync_client.set(None) + if client := self.__sync_client.get(): + try: + client.close() + finally: + self.__sync_client.set(None) # async context async def __aenter__(self): @@ -238,8 +241,11 @@ async def __aexit__( exc_value: BaseException | None = None, traceback: TracebackType | None = None, ): - await cast(httpx.AsyncClient, self.__async_client.get()).aclose() - self.__async_client.set(None) + if client := self.__async_client.get(): + try: + await client.aclose() + finally: + self.__async_client.set(None) def _get_client_defaults(self) -> dict[str, Any]: """Get default arguments for creating a httpx client.""" diff --git a/githubkit/throttling.py b/githubkit/throttling.py index ab22afe208..15bfd5e626 100644 --- a/githubkit/throttling.py +++ b/githubkit/throttling.py @@ -37,19 +37,24 @@ class LocalThrottler(BaseThrottler): def __init__(self, max_concurrency: int) -> None: self.max_concurrency = max_concurrency + self._lock = threading.Lock() self._semaphore: threading.Semaphore | None = None self._async_semaphore: anyio.Semaphore | None = None @property def semaphore(self) -> threading.Semaphore: if self._semaphore is None: - self._semaphore = threading.Semaphore(self.max_concurrency) + with self._lock: + if self._semaphore is None: + self._semaphore = threading.Semaphore(self.max_concurrency) return self._semaphore @property def async_semaphore(self) -> anyio.Semaphore: if self._async_semaphore is None: - self._async_semaphore = anyio.Semaphore(self.max_concurrency) + with self._lock: + if self._async_semaphore is None: + self._async_semaphore = anyio.Semaphore(self.max_concurrency) return self._async_semaphore @override diff --git a/tests/test_unit_test/test_unit_test.py b/tests/test_unit_test/test_unit_test.py index e989f3808a..86892f9bf6 100644 --- a/tests/test_unit_test/test_unit_test.py +++ b/tests/test_unit_test/test_unit_test.py @@ -1,13 +1,17 @@ import json from pathlib import Path +import threading +from datetime import timedelta from typing import Any, TypeVar from githubkit_schemas.latest.models import FullRepository import httpx import pytest -from githubkit import GitHub +from githubkit import GitHub, GitHubCore +from githubkit.cache.mem_cache import MemCache from githubkit.response import Response +from githubkit.throttling import LocalThrottler from githubkit.typing import UnsetType, URLTypes from githubkit.utils import UNSET @@ -76,3 +80,49 @@ async def test_async_mock(): repo = await target_async_func() assert isinstance(repo, FullRepository) + + +def test_local_throttler_thread_safety(): + throttler = LocalThrottler(max_concurrency=5) + threads = [] + semaphores = [] + + def get_sem(): + semaphores.append(throttler.semaphore) + + for _ in range(20): + t = threading.Thread(target=get_sem) + threads.append(t) + t.start() + + for t in threads: + t.join() + + assert len(semaphores) == 20 + first_sem = semaphores[0] + for sem in semaphores: + assert sem is first_sem + + +def test_mem_cache_passive_expiry(): + cache = MemCache() + cache.set("key1", "val1", timedelta(milliseconds=1)) + cache.set("key2", "val2", timedelta(hours=1)) + + import time + time.sleep(0.01) + + assert cache.get("key1") is None + assert cache.get("key2") == "val2" + assert "key2" in cache._cache + + +def test_core_context_manager_safety(): + gh = GitHubCore() + with pytest.raises(RuntimeError): + with gh: + with gh: + pass + + # Ensure no lingering client after error + assert gh._GitHubCore__sync_client.get() is None From eecad2df846fafef4cf1dfc36fb91978dffe5c5c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:19:26 +0000 Subject: [PATCH 2/3] :rotating_light: auto fix by pre-commit hooks --- githubkit/core.py | 2 +- tests/test_unit_test/test_unit_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/githubkit/core.py b/githubkit/core.py index 108f05625f..82fbf997f1 100644 --- a/githubkit/core.py +++ b/githubkit/core.py @@ -4,7 +4,7 @@ from datetime import datetime, timedelta, timezone import time from types import TracebackType -from typing import TYPE_CHECKING, Any, Generic, TypeVar, cast, overload +from typing import TYPE_CHECKING, Any, Generic, TypeVar, overload import anyio import httpx diff --git a/tests/test_unit_test/test_unit_test.py b/tests/test_unit_test/test_unit_test.py index 86892f9bf6..9579e13f88 100644 --- a/tests/test_unit_test/test_unit_test.py +++ b/tests/test_unit_test/test_unit_test.py @@ -1,7 +1,7 @@ +from datetime import timedelta import json from pathlib import Path import threading -from datetime import timedelta from typing import Any, TypeVar from githubkit_schemas.latest.models import FullRepository @@ -110,6 +110,7 @@ def test_mem_cache_passive_expiry(): cache.set("key2", "val2", timedelta(hours=1)) import time + time.sleep(0.01) assert cache.get("key1") is None From f42bfa77be1c5c7ca2c4f366de7691c063ee1b47 Mon Sep 17 00:00:00 2001 From: AtaCanYmc Date: Tue, 4 Aug 2026 23:15:59 +0300 Subject: [PATCH 3/3] fix(core): resolve throttling race condition and optimize memory cache 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 --- githubkit/cache/mem_cache.py | 10 +++-- githubkit/core.py | 16 +++++--- githubkit/throttling.py | 9 ++++- tests/test_unit_test/test_unit_test.py | 52 +++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/githubkit/cache/mem_cache.py b/githubkit/cache/mem_cache.py index 834378b6b1..38dfaad87f 100644 --- a/githubkit/cache/mem_cache.py +++ b/githubkit/cache/mem_cache.py @@ -27,8 +27,13 @@ def expire(self): @override def get(self, key: str) -> str | None: - self.expire() - return (item := self._cache.get(key, None)) and item.value + item = self._cache.get(key) + if item is None: + return None + if item.expire_at is not None and item.expire_at < datetime.now(timezone.utc): + self._cache.pop(key, None) + return None + return item.value @override async def aget(self, key: str) -> str | None: @@ -36,7 +41,6 @@ async def aget(self, key: str) -> str | None: @override def set(self, key: str, value: str, ex: timedelta) -> None: - self.expire() self._cache[key] = _Item(value, datetime.now(timezone.utc) + ex) @override diff --git a/githubkit/core.py b/githubkit/core.py index 2b5faff162..82fbf997f1 100644 --- a/githubkit/core.py +++ b/githubkit/core.py @@ -4,7 +4,7 @@ from datetime import datetime, timedelta, timezone import time from types import TracebackType -from typing import TYPE_CHECKING, Any, Generic, TypeVar, cast, overload +from typing import TYPE_CHECKING, Any, Generic, TypeVar, overload import anyio import httpx @@ -222,8 +222,11 @@ def __exit__( exc_value: BaseException | None = None, traceback: TracebackType | None = None, ): - cast(httpx.Client, self.__sync_client.get()).close() - self.__sync_client.set(None) + if client := self.__sync_client.get(): + try: + client.close() + finally: + self.__sync_client.set(None) # async context async def __aenter__(self): @@ -238,8 +241,11 @@ async def __aexit__( exc_value: BaseException | None = None, traceback: TracebackType | None = None, ): - await cast(httpx.AsyncClient, self.__async_client.get()).aclose() - self.__async_client.set(None) + if client := self.__async_client.get(): + try: + await client.aclose() + finally: + self.__async_client.set(None) def _get_client_defaults(self) -> dict[str, Any]: """Get default arguments for creating a httpx client.""" diff --git a/githubkit/throttling.py b/githubkit/throttling.py index ab22afe208..15bfd5e626 100644 --- a/githubkit/throttling.py +++ b/githubkit/throttling.py @@ -37,19 +37,24 @@ class LocalThrottler(BaseThrottler): def __init__(self, max_concurrency: int) -> None: self.max_concurrency = max_concurrency + self._lock = threading.Lock() self._semaphore: threading.Semaphore | None = None self._async_semaphore: anyio.Semaphore | None = None @property def semaphore(self) -> threading.Semaphore: if self._semaphore is None: - self._semaphore = threading.Semaphore(self.max_concurrency) + with self._lock: + if self._semaphore is None: + self._semaphore = threading.Semaphore(self.max_concurrency) return self._semaphore @property def async_semaphore(self) -> anyio.Semaphore: if self._async_semaphore is None: - self._async_semaphore = anyio.Semaphore(self.max_concurrency) + with self._lock: + if self._async_semaphore is None: + self._async_semaphore = anyio.Semaphore(self.max_concurrency) return self._async_semaphore @override diff --git a/tests/test_unit_test/test_unit_test.py b/tests/test_unit_test/test_unit_test.py index e989f3808a..455c15831d 100644 --- a/tests/test_unit_test/test_unit_test.py +++ b/tests/test_unit_test/test_unit_test.py @@ -1,13 +1,17 @@ +from datetime import timedelta import json from pathlib import Path +import threading from typing import Any, TypeVar from githubkit_schemas.latest.models import FullRepository import httpx import pytest -from githubkit import GitHub +from githubkit import GitHub, GitHubCore +from githubkit.cache.mem_cache import MemCache from githubkit.response import Response +from githubkit.throttling import LocalThrottler from githubkit.typing import UnsetType, URLTypes from githubkit.utils import UNSET @@ -76,3 +80,49 @@ async def test_async_mock(): repo = await target_async_func() assert isinstance(repo, FullRepository) + + +def test_local_throttler_thread_safety(): + throttler = LocalThrottler(max_concurrency=5) + threads = [] + semaphores = [] + + def get_sem(): + semaphores.append(throttler.semaphore) + + for _ in range(20): + t = threading.Thread(target=get_sem) + threads.append(t) + t.start() + + for t in threads: + t.join() + + assert len(semaphores) == 20 + first_sem = semaphores[0] + for sem in semaphores: + assert sem is first_sem + + +def test_mem_cache_passive_expiry(): + cache = MemCache() + cache.set("key1", "val1", timedelta(milliseconds=1)) + cache.set("key2", "val2", timedelta(hours=1)) + + import time + + time.sleep(0.01) + + assert cache.get("key1") is None + assert cache.get("key2") == "val2" + assert "key2" in cache._cache + + +def test_core_context_manager_safety(): + gh = GitHubCore() + with gh: + with pytest.raises(RuntimeError): + gh.__enter__() + + # Ensure no lingering client after error + assert gh._GitHubCore__sync_client.get() is None