LanguageServerWrapper is not stopped on Eclipse shutdown

[basilevs]

Eclipse outputs the following error to standard error on shutdown, if any LSP-enabled editors are still opened:

Jun 04, 2023 8:12:12 PM org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer fireStreamClosed
INFO: The input stream was closed.
java.io.InterruptedIOException
	at java.base/java.io.PipedInputStream.read(PipedInputStream.java:328)
	at org.vgcpge.eclipse.copilot.ui.internal.OrphanPipedInputStream.read(OrphanPipedInputStream.java:21)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:79)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

The problem is caused by interruption of the MessageProducer on shutdown.

Thread [Framework stop - Equinox Container: e7e68fc9-1058-4290-853a-605fd888929e] (Suspended (breakpoint at line 670 in ThreadPoolExecutor$Worker))	
	owns: Object  (id=178)	
	ThreadPoolExecutor$Worker.interruptIfStarted() line: 670	
	ThreadPoolExecutor.interruptWorkers() line: 769	
	ThreadPoolExecutor.shutdownNow() line: 1417	
	LanguageServerWrapper.stopDispatcher() line: 210	
	0x00000008013a7440.accept(Object) line: not available	
	CopyOnWriteArrayList<E>.forEach(Consumer<? super E>) line: 807	
	CopyOnWriteArraySet<E>.forEach(Consumer<? super E>) line: 425	
	LanguageServiceAccessor.shutdownAllDispatchers() line: 686	
	LanguageServerPlugin.stop(BundleContext) line: 43	
	BundleContextImpl$3.run() line: 875	
	BundleContextImpl$3.run() line: 1	
	AccessController.executePrivileged(PrivilegedExceptionAction<T>, AccessControlContext, Class<?>) line: 807	
	AccessController.doPrivileged(PrivilegedExceptionAction<T>) line: 569	
	BundleContextImpl.stop() line: 867	
	EquinoxBundle.stopWorker0() line: 1046	
	EquinoxBundle$EquinoxModule.stopWorker() line: 376	
	EquinoxBundle$EquinoxModule(Module).doStop() line: 660	
	EquinoxBundle$EquinoxModule(Module).stop(Module$StopOptions...) line: 521	
	ModuleContainer$ContainerStartLevel.decStartLevel(int, List<Module>) line: 1893	
	ModuleContainer$ContainerStartLevel.doContainerStartLevel(Module, int, FrameworkListener...) line: 1768	
	EquinoxBundle$SystemBundle$EquinoxSystemModule(SystemModule).stopWorker() line: 275	
	EquinoxBundle$SystemBundle$EquinoxSystemModule.stopWorker() line: 208	
	EquinoxBundle$SystemBundle$EquinoxSystemModule(Module).doStop() line: 660	
	EquinoxBundle$SystemBundle$EquinoxSystemModule(Module).stop(Module$StopOptions...) line: 521	
	EquinoxBundle$SystemBundle$EquinoxSystemModule(SystemModule).stop(Module$StopOptions...) line: 207	
	EquinoxBundle$SystemBundle$EquinoxSystemModule$1.run() line: 226	
	Thread.run() line: 833	

Investigation shows, that LSP4E relies on LanguageServerWrapper to stop processing before its Executor is shutdown. However, the only way to stop processing is to call LanguageServerWrapper.stop() and that is not called on shutdown - the call is delayed too much by org.eclipse.lsp4e.LanguageServerWrapper.startStopTimer().

AFAIK, this results in forceful termination of child process without a chance for graceful shutdown.

[basilevs]

Why does org.eclipse.lsp4e.LanguageServerPlugin.stop(BundleContext) use org.eclipse.lsp4e.LanguageServiceAccessor.shutdownAllDispatchers() and not org.eclipse.lsp4e.LanguageServiceAccessor.clearStartedServers()? It would not solve the problem, as org.eclipse.lsp4e.LanguageServerWrapper.stop() is asynchronous in nature, but would be a good first step.

[mickaelistria]

@basilevs your proposal sounds good. Can you please submit a PR about that?

[rubenporras]

Looks good to me as well. @mickaelistria , would we then remove shutdownAllDispatchers? As far as I can tell, the method would not be used any longer.

[basilevs]

I no longer believe that a simple change will help. Stop process needs to be monitored and completed before shutdown of the executors.

The new suggested solution is:

• make stop synchronous
• compensate for this in org.eclipse.lsp4e.LanguageServerWrapper.start() with an asyncExec and additional promise indirection to ensure start() is still non-blocking
• call stop from org.eclipse.lsp4e.LanguageServerWrapper.stopDispatcher() before shutting down Executors

I will work on a PR, but ETA is unclear.

UPDATE:
This approach has failed due to a requirement for stop() to quickly update part of LanguageServerWrapper state. A new approach is described and implemented in #688.

[basilevs]

org.eclipse.lsp4e.LanguageServerWrapper.start() fails to properly manage mutexes - the method in synchronized, but accesses class internals from a background thread without mutex protections.

[basilevs]

Well, this did not work out - the quick acting stop is required during disconnect, so I had to revert synchronous stop.

[basilevs]

#688 is ready for review and seems to fix the described problem.