openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
546 stars 101 forks source link

pygls does not handle async user `shutdown` handler correctly #433

Open alcarney opened 5 months ago

alcarney commented 5 months ago

If I register an async handler for the shutdown request, pygls does not guarantee that it will finish executing before responding to the client. This often leads to the client sending the exit notification and pygls calling sys.exit() before my code cleaned up everything it needs to.

I'm finding this is leading to a lot of orphaned esbonio processes hanging around on my machine! :sweat_smile: image

I think the issue is down to how pygls handles the user registering features that "shadow" built in features.

https://github.com/openlawlibrary/pygls/blob/d3a14210c0d892bca4c2101f3f2cf9b7c843e2a3/pygls/protocol/lsp_meta.py#L15-L29

If we look at how _execute_notification is implemented

https://github.com/openlawlibrary/pygls/blob/d3a14210c0d892bca4c2101f3f2cf9b7c843e2a3/pygls/protocol/json_rpc.py#L144-L153

It uses asyncio.ensure_future to schedule the coroutine to be executed, but then will return almost immediately - meaning there's no guarantee it will complete before pygls responds to the client.

alcarney commented 5 months ago

I think I have an idea that will fix this, but it will probably have to go into #418.

If we allow pygls' built-in features to yield, there's an opportunity to run the user's code in the middle of the method, before resuming the built-in to run to completion.

Not only would this fix this issue, but it might help with #381, where pygls can do some initial setup e.g. initialize the workspace, set client capabilities on the server object etc. and then yield to the user's code - allowing them to register some extra features, before resuming to compute the server's capabilities and eventually respond to the client