lsp4clj
lsp4clj copied to clipboard
Complete pending server-initiated request promises on default executor
There's a subtle detail on Promesa promises (CompletableFuture): Unless an executor is specified, callbacks will be executed on the thread that completed the promise.
For lsp4clj, this means that users need to be careful when using send-request and coercing the result to a Promesa promise: The code run in handlers such as p/then or p/catch will end up running in a core.async dispatch thread, as the promise is completed from within a go-loop.
Since go blocks are executed in a fixed-size thread pool, one should not block in go blocks. Consequently, users should not block in code that handles the send-request promise, unless they explicitly move it to a different executor.
This is easy to get wrong, especially with Promesa's convenience macros like p/let, which will coerce the PendingRequest to a promise without a dedicated executor. Furthermore, Promesa does not expose CompletionStage.exceptionallyAsync(), i.e. it does not support specifying an executor with p/catch, therefore it is not trivial to ensure all continuation happens in another thread.
This PR attempts to improve this by completing the promise in a thread from the :default executor ((ForkJoinPool/commonPool)), instead of the calling thread.
Some caveats to this approach:
- There's some overhead even when the promise does not have any continuation handlers (apart from the
CancellationExceptionwe handle ourselves, which, since it blocks on asend-notificationcall, should also run on a suitable executor) - We don't allow users to customise the executor, but use promesa's default, which is
(ForkJoinPool/commonPool). Users might prefer to use a virtual thread executor instead. We could add a:response-executoroption for this?
The change to .cljfmt.edn is unrelated, but required for more recent version of cljfmt, as used by Calva, for example. This could be moved to a different PR of course.