diff --git a/changelog/1585.bugfix.rst b/changelog/1585.bugfix.rst new file mode 100644 index 0000000000..e38df25f3b --- /dev/null +++ b/changelog/1585.bugfix.rst @@ -0,0 +1 @@ +Fix a deadlock where cancelling a request while it was waiting out a global rate limit would leave every subsequent request waiting forever. diff --git a/disnake/http.py b/disnake/http.py index b9bca14ae6..d682dbc06a 100644 --- a/disnake/http.py +++ b/disnake/http.py @@ -409,14 +409,19 @@ async def request( ) self._global_over.clear() - await asyncio.sleep(retry_after) - _log.debug("Done sleeping for the rate limit. Retrying...") - - # release the global lock now that the - # global rate limit has passed - if is_global: - self._global_over.set() - _log.debug("Global rate limit is now over.") + try: + await asyncio.sleep(retry_after) + _log.debug("Done sleeping for the rate limit. Retrying...") + finally: + # release the global lock now that the + # global rate limit has passed. + # this has to happen in a finally, otherwise a + # request cancelled while sleeping would leave + # the event cleared forever, and every later + # request would block on it indefinitely + if is_global: + self._global_over.set() + _log.debug("Global rate limit is now over.") continue diff --git a/tests/test_http.py b/tests/test_http.py index 6a9512a287..ba05f41eba 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -1,9 +1,13 @@ # SPDX-License-Identifier: MIT +import asyncio +from typing import ClassVar + import pytest +from typing_extensions import Self import disnake -from disnake.http import HTTPClient +from disnake.http import HTTPClient, Route @pytest.mark.parametrize( @@ -46,3 +50,50 @@ ) def test_format_gateway_url(url: str, params: disnake.GatewayParams, expected: str) -> None: assert HTTPClient._format_gateway_url(url, params=params) == expected + + +class _GlobalRateLimitResponse: + """A ``429`` carrying ``global: true``, as sent by Discord.""" + + status = 429 + headers: ClassVar[dict[str, str]] = { + "content-type": "application/json", + "Via": "1.1 google", + } + + async def text(self, encoding: str = "utf-8") -> str: + return '{"global": true, "retry_after": 60.0}' + + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *exc_info: object) -> bool: + return False + + +class _GlobalRateLimitSession: + def request(self, method: str, url: str, **kwargs: object) -> _GlobalRateLimitResponse: + return _GlobalRateLimitResponse() + + +@pytest.mark.asyncio +async def test_global_rate_limit_released_on_cancellation() -> None: + # a request cancelled while sleeping off a global rate limit must still + # re-set `_global_over`, otherwise every later request blocks on it forever + http = HTTPClient(loop=asyncio.get_running_loop()) + http._HTTPClient__session = _GlobalRateLimitSession() # pyright: ignore[reportAttributeAccessIssue] + + task = asyncio.create_task(http.request(Route("GET", "/users/@me"))) + + # let the request reach the sleep, with the global event now cleared + for _ in range(10): + await asyncio.sleep(0) + if not http._global_over.is_set(): + break + assert not http._global_over.is_set() + + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + assert http._global_over.is_set()