From ed387b42561a50f3b49f6420fd993c6e9f3f3fe8 Mon Sep 17 00:00:00 2001 From: Alex Carney Date: Sun, 24 Nov 2024 18:44:05 +0000 Subject: [PATCH] lsp: Use `concurrent.futures.Future` instead of `asyncio.Future` pygls in `v2.0a2` switched from using the low-level asyncio APIs to using the high-level one and surprisingly, this broke `esbonio`. The server would start ok, but wouldn't launch Sphinx processes, the preview command did nothing, it wouldn't even produce any log messages! That is, until the user changed a configuration setting. After changing a setting - any setting, the server suddenly springs into life, logs, previews, sphinx processes all of them would start working as if nothing was wrong. I eventually managed to figure out that the callbacks registered by the configuration system on the server's `ready` future were never being called. ``` self.server.ready.add_done_callback(self._notify_subscriptions) ``` By why would changing the asyncio API in use break these callbacks?! After spending some time with the debugger I eventually spotted the culprit ``` >>> asyncio.get_running_loop() <_UnixSelectorEventLoop running=True closed=False debug=False> >>> self.server.ready._loop <_UnixSelectorEventLoop running=False closed=False debug=False> ``` The server's `ready` future was using a different event loop and because the event loop is not running, when the future is resolved, the callbacks were never scheduled! While I cannot explain why this was not an issue before, I can explain why it is an issue now. The `ready` future is created in the constructor of the `EsbonioLanguageServer` class - before any event loop has been created and so it uses a new one based on the current event loop policy[1][2]. Unfortunately, when pygls later starts its main loop by calling `asyncio.run()` it creates a new event loop, orphaning the `ready` future in its unused event loop[3] Since we don't need to use the `ready` future asynchronously, the simplest fix is to convert it to future from the `concurrent.futures` module, removing the need for an event loop completely. [1]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/futures.py#L79 [2]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/events.py#L791 [3]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/runners.py#L146 --- lib/esbonio/esbonio/server/server.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/esbonio/esbonio/server/server.py b/lib/esbonio/esbonio/server/server.py index e21957ae..e177fcba 100644 --- a/lib/esbonio/esbonio/server/server.py +++ b/lib/esbonio/esbonio/server/server.py @@ -7,6 +7,7 @@ import platform import traceback import typing +from concurrent.futures import Future from typing import TypeVar from uuid import uuid4 @@ -79,7 +80,7 @@ def __init__(self, logger: logging.Logger | None = None, *args, **kwargs): self._features: dict[type[LanguageFeature], LanguageFeature] = {} """The collection of language features registered with the server.""" - self._ready: asyncio.Future[bool] = asyncio.Future() + self._ready: Future[bool] = Future() """Indicates if the server is ready.""" self._tasks: set[asyncio.Task] = set() @@ -95,7 +96,7 @@ def __iter__(self): return iter(self._features.items()) @property - def ready(self) -> asyncio.Future: + def ready(self) -> Future[bool]: return self._ready @property