Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change Launcher startListening result to CompletableFuture? #575

Open
frankbenoit opened this issue Nov 28, 2021 · 1 comment
Open

change Launcher startListening result to CompletableFuture? #575

frankbenoit opened this issue Nov 28, 2021 · 1 comment

Comments

@frankbenoit
Copy link

frankbenoit commented Nov 28, 2021

Hi Lsp4J,

I wonder if it would make sense to have a CompletableFuture as the result type of the JSON-RPC Launcher#startListening?
The Future is create here:
org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.startProcessing(MessageProducer, MessageConsumer, ExecutorService)
org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.beginProcessing(ExecutorService)
Could it be changed to CompletableFuture.runAsync( reader/this, executorService )

In the ChatExample from here: https://www.eclipse.org/community/eclipse_newsletter/2017/may/article2.php
The class SocketLauncher is mainly there to do the conversion Future -> CompletableFuture and creates a blocking thread to do so. This seems waste to me.

Frank

@pisv
Copy link
Contributor

pisv commented Nov 28, 2021

Hi Frank,

Thank you for your suggestion, it sounds interesting.

Until someone with a deeper knowledge of the subject replies, here are my two cents:

  1. Obviously, this would be a breaking API change. In particular, it could break any existing implementation of the Launcher interface.

  2. There is a difference in behavior of the cancel method of a Future returned from ExecutorService.submit and a CompletableFuture returned from CompletableFuture.runAsyc. The latter ignores the mayInterruptIfRunning parameter, i.e. it will not try to stop the running task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants