Opening files in SyncLanguageServer may cause deadlocks

[irreg]

A quick investigation revealed that while start_server launches processing in a separate thread, open_file itself is not launched in the thread. The issue was resolved by executing open_file's processing in the thread, suggesting that internally, there may be operations that are not thread-safe.
I haven't tested all languages, but it occurs at least for Java and C++.
SyncLanguageServer should be modified to completely hide the existence of separate threads.

Environment

OS: Windows 11
Python: 3.13

Reproduction Code

Freezes within 10 times

from pathlib import Path

from multilspy import SyncLanguageServer
from multilspy.multilspy_config import MultilspyConfig
from multilspy.multilspy_logger import MultilspyLogger


def main():
    path = Path("temp")
    path.mkdir(exist_ok=True)
    with open(path / "test.java", "w"):
        pass
    params = {
        "code_language": "java",
    }
    config = MultilspyConfig.from_dict(params)
    logger = MultilspyLogger()
    for i in range(100):
        print(f"start: {i}")
        lsp = SyncLanguageServer.create(config, logger, str(path.absolute()))

        with lsp.start_server():
            with lsp.open_file("test.java"):
                pass
            # Changing the processing as described in the comment-out will avoid the problem.
            # async def sub():
            #     with lsp.open_file("test.java"):
            #         pass

            # asyncio.run_coroutine_threadsafe(sub(), lsp.loop).result(
            #     timeout=lsp.timeout
            # )
        print(f"end: {i}")


if __name__ == "__main__":
    main()

I don't fully understand the internal processing, but the following fixes are likely candidates:

    @contextmanager
    def open_file(self, relative_file_path: str) -> Iterator[None]:
        """
        Open a file in the Language Server. This is required before making any requests to the Language Server.

        :param relative_file_path: The relative path of the file to open.
        """
        f = self.language_server.open_file(relative_file_path)
        async def enter():
            f.__enter__()
        async def exit():
            f.__exit__(None, None, None)
        asyncio.run_coroutine_threadsafe(enter(), lsp.loop).result(
             timeout=lsp.timeout
        )
        try:
            yield
        finally:
           asyncio.run_coroutine_threadsafe(exit(), lsp.loop).result(
             timeout=lsp.timeout
           )

[MischaPanch]

@irreg just FYI - we encountered unsurmountable race conditions and could only address them by removing all async code and rewriting multilspy as a sync-first library. If you are interested, you'll find it here.

[irreg]

@MischaPanch
Thank you for letting me know.

One point I noticed: It seems the issue I reported to Multilspy also exists in the code you shared, so you might want to check it out.
fix: finalize even when exiting contextmanager by exception

[MischaPanch]

Oh, thanks for the pointer, I'll have a look where we need to adjust it in our code!

[observerw]

Hi there! Dealing with deadlocks in synchronous wrappers for async LSP clients can be tricky. We've been working on lsp-client with a focus on reliable async/await patterns and thread safety. If you're looking for an alternative or want to help us improve the state of LSP clients in Python, we'd love for you to try it out and share your feedback!

[MischaPanch]

@observerw

What's the benefit of having async for this? The language server is running in stdio mode and can only process things sequentially. Async can't bring a performance benefit, unless I'm misunderstanding something.

Note that within Serena we are maintaining the solidlsp package which can be used as a python library, where already around 30 language servers are supported, and which is fully synchronous, thus circumventing any race conditions and deadlocks

[observerw]

@observerw

What's the benefit of having async for this? The language server is running in stdio mode and can only process things sequentially. Async can't bring a performance benefit, unless I'm misunderstanding something.

Note that within Serena we are maintaining the solidlsp package which can be used as a python library, where already around 30 language servers are supported, and which is fully synchronous, thus circumventing any race conditions and deadlocks

Thank you for sharing your opinion. AFAIK language servers are not "guaranteed" to be sequential (although that is the case most of the time), so it would be beneficial for the client side to be async/multi-threaded. Sure we could use multiprocessing, but it would be much easier to just use async, which can avoid some process synchronization problems and provide potential performance benefits (pyrefly for example). I believe the issue does not exist if we don't bother with asyncio's thread-safe behavior.

Serena is great, big fan of it. However I can't wrap my head around why to make everything "fully synchronous" (maybe I need to dig deeper to understand).