Race condition in LanguageServerWrapper on exception in StreamConnectionProvider.start()

[ChristophKaser]

Hi everyone,

I have observed the following behavior when I use a custom StreamConnectionProvider that tries to contact a remote language server using websockets: When the server is down, eventually lsp4e stops trying to recreate the language server connection. This is caused by an exception thrown in StreamConnectionProvider.start() that causes the following race condition:

• In

  lsp4e/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerWrapper.java

  Line 326 in 9cdd9be

  stop();

  the start()-exception is handled by calling stop(), which sets inititalizeFuture = null
• In

  lsp4e/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerWrapper.java

  Line 259 in 9cdd9be

  this.initializeFuture = CompletableFuture.supplyAsync(() -> {

  initializeFuture is assigned the value from CompletableFuture.supplyAsync, which might happen before or after the start()-Exception has been handled.

Sometimes, the initializeFuture gets assigned the failed future from line 259 after the stop-Method has been called in line 326. When this happens, LanguageServerWrapper.start() does not start a new language server any more, because it only checks for initializeFuture == null in

lsp4e/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerWrapper.java

Line 256 in 9cdd9be

if (this.initializeFuture == null) {

.

One possible solution might be to amend line 256 with something like && !this.initializeFuture.isCompletedExceptionally().

[mickaelistria]

Thanks for the PR. Feel free to submit a PR and we'll review it.
If you can manage to reproduce this case with a unit test as part of the PR, it would be great!

[ChristophKaser]

I created a PR with a unit test that reproduces the problem reliably for me and a fix.