You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@netbeans.apache.org by ge...@apache.org on 2021/04/16 10:00:40 UTC
[netbeans] branch master updated: When LSP Server fails to start
too many times, don't try start it again.
This is an automated email from the ASF dual-hosted git repository.
geertjan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/netbeans.git
The following commit(s) were added to refs/heads/master by this push:
new 281781d When LSP Server fails to start too many times, don't try start it again.
new 991e439 Merge pull request #2870 from jlahoda/lsp-server-failed-to-start
281781d is described below
commit 281781d091b76bd93433c3c678090a7142f1ab10
Author: Jan Lahoda <jl...@netbeans.org>
AuthorDate: Mon Apr 5 23:33:35 2021 +0200
When LSP Server fails to start too many times, don't try start it again.
---
.../cpplite/editor/lsp/LanguageServerImpl.java | 12 +-
.../netbeans/modules/lsp/client/LSPBindings.java | 71 ++++++++---
.../modules/lsp/client/resources/error_16.png | Bin 0 -> 730 bytes
.../modules/lsp/client/LSPBindingsTest.java | 140 +++++++++++++++++++--
4 files changed, 196 insertions(+), 27 deletions(-)
diff --git a/cpplite/cpplite.editor/src/org/netbeans/modules/cpplite/editor/lsp/LanguageServerImpl.java b/cpplite/cpplite.editor/src/org/netbeans/modules/cpplite/editor/lsp/LanguageServerImpl.java
index 27a3971..877c619 100644
--- a/cpplite/cpplite.editor/src/org/netbeans/modules/cpplite/editor/lsp/LanguageServerImpl.java
+++ b/cpplite/cpplite.editor/src/org/netbeans/modules/cpplite/editor/lsp/LanguageServerImpl.java
@@ -47,6 +47,7 @@ import org.netbeans.modules.cpplite.editor.spi.CProjectConfigurationProvider;
import org.netbeans.modules.cpplite.editor.spi.CProjectConfigurationProvider.ProjectConfiguration;
import org.openide.filesystems.FileUtil;
import org.openide.modules.Places;
+import org.openide.util.Pair;
/**
*
@@ -66,7 +67,7 @@ public class LanguageServerImpl implements LanguageServerProvider {
private static final Logger LOG = Logger.getLogger(LanguageServerImpl.class.getName());
- private static final Map<Project, LanguageServerDescription> prj2Server = new HashMap<>();
+ private static final Map<Project, Pair<Process, LanguageServerDescription>> prj2Server = new HashMap<>();
@Override
public LanguageServerDescription startServer(Lookup lookup) {
@@ -88,7 +89,10 @@ public class LanguageServerImpl implements LanguageServerProvider {
String ccls = Utils.getCCLSPath();
String clangd = Utils.getCLANGDPath();
if (ccls != null || clangd != null) {
- return prj2Server.computeIfAbsent(prj, (Project p) -> {
+ return prj2Server.compute(prj, (p, pair) -> {
+ if (pair != null && pair.first().isAlive()) {
+ return pair;
+ }
try {
List<String> command = new ArrayList<>();
@@ -128,14 +132,14 @@ public class LanguageServerImpl implements LanguageServerProvider {
in = new CopyInput(in, System.err);
out = new CopyOutput(out, System.err);
}
- return LanguageServerDescription.create(in, out, process);
+ return Pair.of(process, LanguageServerDescription.create(in, out, process));
}
return null;
} catch (IOException ex) {
LOG.log(Level.FINE, null, ex);
return null;
}
- });
+ }).second();
}
return null;
}
diff --git a/ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java b/ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
index 6744c6d..06ce534 100644
--- a/ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
+++ b/ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
@@ -22,6 +22,7 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.net.InetAddress;
@@ -74,11 +75,13 @@ import org.netbeans.modules.lsp.client.options.MimeTypeInfo;
import org.netbeans.modules.lsp.client.spi.ServerRestarter;
import org.netbeans.modules.lsp.client.spi.LanguageServerProvider;
import org.netbeans.modules.lsp.client.spi.LanguageServerProvider.LanguageServerDescription;
+import org.openide.awt.NotificationDisplayer;
import org.openide.filesystems.FileObject;
import org.openide.filesystems.FileUtil;
import org.openide.modules.OnStop;
import org.openide.util.ChangeSupport;
import org.openide.util.Exceptions;
+import org.openide.util.ImageUtilities;
import org.openide.util.Lookup;
import org.openide.util.NbBundle.Messages;
import org.openide.util.RequestProcessor;
@@ -95,10 +98,12 @@ public class LSPBindings {
private static final int DELAY = 500;
private static final int LSP_KEEP_ALIVE_MINUTES = 10;
+ private static final int INVALID_START_TIME = 1 * 60 * 1000;
+ private static final int INVALID_START_MAX_COUNT = 5;
private static final RequestProcessor WORKER = new RequestProcessor(LanguageClientImpl.class.getName(), 1, false, false);
private static final ChangeSupport cs = new ChangeSupport(LSPBindings.class);
private static final Map<LSPBindings,Long> lspKeepAlive = new IdentityHashMap<>();
- private static final Map<URI, Map<String, WeakReference<LSPBindings>>> project2MimeType2Server = new HashMap<>();
+ private static final Map<URI, Map<String, ServerDescription>> project2MimeType2Server = new HashMap<>();
private static final Map<FileObject, Map<String, LSPBindings>> workspace2Extension2Server = new HashMap<>();
static {
@@ -168,23 +173,28 @@ public class LSPBindings {
URI uri = dir.toURI();
LSPBindings bindings = null;
- WeakReference<LSPBindings> bindingsReference =
+ ServerDescription description =
project2MimeType2Server.computeIfAbsent(uri, p -> new HashMap<>())
- .get(mimeType);
+ .computeIfAbsent(mimeType, m -> new ServerDescription());
- if(bindingsReference != null) {
- bindings = bindingsReference.get();
+ if (description.bindings != null) {
+ bindings = description.bindings.get();
}
if (bindings != null && bindings.process != null && !bindings.process.isAlive()) {
+ startFailed(description, mimeType);
bindings = null;
}
+ if (description.failedCount >= INVALID_START_MAX_COUNT) {
+ return null;
+ }
+
if (bindings == null) {
- bindings = buildBindings(prj, mimeType, dir, uri);
+ bindings = buildBindings(description, prj, mimeType, dir, uri);
if (bindings != null) {
- project2MimeType2Server.computeIfAbsent(uri, p -> new HashMap<>())
- .put(mimeType, new WeakReference<>(bindings));
+ description.bindings = new WeakReference<>(bindings);
+ description.lastStartTimeStamp = System.currentTimeMillis();
WORKER.post(() -> cs.fireChange());
}
}
@@ -196,12 +206,34 @@ public class LSPBindings {
return bindings != null ? bindings : null;
}
+ @Messages({
+ "# {0} - the mime type for which the LSP server failed to start",
+ "TITLE_FailedToStart=LSP Server for {0} failed to start too many times.",
+ "DETAIL_FailedToStart=The LSP Server failed to start too many times in a short time, and will not be restarted anymore."
+ })
+ private static void startFailed(ServerDescription description, String mimeType) {
+ long timeStamp = System.currentTimeMillis();
+ if (timeStamp - description.lastStartTimeStamp < INVALID_START_TIME) {
+ description.failedCount++;
+ if (description.failedCount == INVALID_START_MAX_COUNT) {
+ NotificationDisplayer.getDefault().notify(Bundle.TITLE_FailedToStart(mimeType),
+ ImageUtilities.loadImageIcon("/org/netbeans/modules/lsp/client/resources/error_16.png", false),
+ Bundle.DETAIL_FailedToStart(),
+ null);
+ }
+ } else {
+ description.failedCount = 0;
+ }
+ description.lastStartTimeStamp = timeStamp;
+ }
+
@SuppressWarnings({"AccessingNonPublicFieldOfAnotherObject", "ResultOfObjectAllocationIgnored"})
- private static LSPBindings buildBindings(Project prj, String mt, FileObject dir, URI baseUri) {
+ private static LSPBindings buildBindings(ServerDescription inDescription, Project prj, String mt, FileObject dir, URI baseUri) {
MimeTypeInfo mimeTypeInfo = new MimeTypeInfo(mt);
ServerRestarter restarter = () -> {
synchronized (LSPBindings.class) {
- WeakReference<LSPBindings> bRef = project2MimeType2Server.getOrDefault(baseUri, Collections.emptyMap()).remove(mt);
+ ServerDescription description = project2MimeType2Server.getOrDefault(baseUri, Collections.emptyMap()).remove(mt);
+ Reference<LSPBindings> bRef = description != null ? description.bindings : null;
LSPBindings b = bRef != null ? bRef.get() : null;
if (b != null) {
@@ -219,6 +251,8 @@ public class LSPBindings {
}
};
+ boolean foundServer = false;
+
for (LanguageServerProvider provider : MimeLookup.getLookup(mt).lookupAll(LanguageServerProvider.class)) {
final Lookup lkp = prj != null ? Lookups.fixed(prj, mimeTypeInfo, restarter) : Lookups.fixed(mimeTypeInfo, restarter);
LanguageServerDescription desc = provider.startServer(lkp);
@@ -228,6 +262,7 @@ public class LSPBindings {
if (b != null) {
return b;
}
+ foundServer = true;
try {
LanguageClientImpl lci = new LanguageClientImpl();
InputStream in = LanguageServerProviderAccessor.getINSTANCE().getInputStream(desc);
@@ -249,6 +284,9 @@ public class LSPBindings {
}
}
}
+ if (foundServer) {
+ startFailed(inDescription, mt);
+ }
return null;
}
@@ -328,7 +366,7 @@ public class LSPBindings {
project2MimeType2Server.values()
.stream()
.flatMap(n -> n.values().stream())
- .map(bindingRef -> bindingRef.get())
+ .map(description -> description.bindings.get())
.filter(binding -> binding != null)
.forEach(allBindings::add);
workspace2Extension2Server.values()
@@ -427,9 +465,9 @@ public class LSPBindings {
@Override
@SuppressWarnings("AccessingNonPublicFieldOfAnotherObject")
public void run() {
- for (Map<String, WeakReference<LSPBindings>> mime2Bindings : project2MimeType2Server.values()) {
- for (WeakReference<LSPBindings> bRef : mime2Bindings.values()) {
- LSPBindings b = bRef != null ? bRef.get() : null;
+ for (Map<String, ServerDescription> mime2Bindings : project2MimeType2Server.values()) {
+ for (ServerDescription description : mime2Bindings.values()) {
+ LSPBindings b = description.bindings != null ? description.bindings.get() : null;
if (b != null && b.process != null) {
b.process.destroy();
}
@@ -488,4 +526,9 @@ public class LSPBindings {
}
}
+ private static class ServerDescription {
+ public long lastStartTimeStamp;
+ public int failedCount;
+ public Reference<LSPBindings> bindings;
+ }
}
diff --git a/ide/lsp.client/src/org/netbeans/modules/lsp/client/resources/error_16.png b/ide/lsp.client/src/org/netbeans/modules/lsp/client/resources/error_16.png
new file mode 100644
index 0000000..7d0ea44
Binary files /dev/null and b/ide/lsp.client/src/org/netbeans/modules/lsp/client/resources/error_16.png differ
diff --git a/ide/lsp.client/test/unit/src/org/netbeans/modules/lsp/client/LSPBindingsTest.java b/ide/lsp.client/test/unit/src/org/netbeans/modules/lsp/client/LSPBindingsTest.java
index a23f42b..619dfa7 100644
--- a/ide/lsp.client/test/unit/src/org/netbeans/modules/lsp/client/LSPBindingsTest.java
+++ b/ide/lsp.client/test/unit/src/org/netbeans/modules/lsp/client/LSPBindingsTest.java
@@ -18,34 +18,44 @@
*/
package org.netbeans.modules.lsp.client;
+import java.awt.event.ActionListener;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.swing.Icon;
+import javax.swing.JComponent;
import static junit.framework.TestCase.assertNotNull;
-import org.eclipse.lsp4j.ServerCapabilities;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import org.junit.Test;
import org.netbeans.api.editor.mimelookup.MimePath;
-import org.netbeans.api.editor.mimelookup.MimeRegistration;
import org.netbeans.api.project.FileOwnerQuery;
-import org.netbeans.api.project.Project;
import org.netbeans.junit.MockServices;
import org.netbeans.modules.editor.mimelookup.MimeLookupCacheSPI;
import org.netbeans.modules.lsp.client.spi.LanguageServerProvider;
+import org.openide.awt.Notification;
+import org.openide.awt.NotificationDisplayer;
import org.openide.filesystems.FileObject;
import org.openide.filesystems.FileUtil;
import org.openide.filesystems.MIMEResolver;
import org.openide.util.Lookup;
import org.openide.util.lookup.Lookups;
+import org.openide.util.lookup.ProxyLookup;
+import org.openide.util.lookup.ServiceProvider;
public class LSPBindingsTest {
+ private static final SettableProxyLookup MIME_LOOKUP = new SettableProxyLookup();
+
@Test
public void testGetBindingsOnNonProjectFolder() throws Exception {
MockServices.setServices(MockMimeLookupCacheSPI.class, MockMimeResolver.class);
+ MIME_LOOKUP.setLookup(Lookups.fixed(new MockLSP()));
+
FileObject folder = FileUtil.createMemoryFileSystem().getRoot().createFolder("myfolder");
FileObject file = folder.createData("data.mock-txt");
assertEquals("application/mock-txt", file.getMIMEType());
@@ -55,11 +65,17 @@ public class LSPBindingsTest {
assertNotNull("Bindings for the projectless file found", bindings);
}
+ public static final class SettableProxyLookup extends ProxyLookup {
+ public void setLookup(Lookup l) {
+ setLookups(l);
+ }
+ }
+
public static final class MockMimeLookupCacheSPI extends MimeLookupCacheSPI {
@Override
public Lookup getLookup(MimePath mp) {
assertEquals("application/mock-txt", mp.getPath());
- return Lookups.fixed(new MockLSP());
+ return MIME_LOOKUP;
}
}
@@ -82,11 +98,11 @@ public class LSPBindingsTest {
}
}
- static final class MockProcess extends Process {
+ static class BaseMockProcess extends Process {
final ByteArrayInputStream in;
final ByteArrayOutputStream out;
- public MockProcess() {
+ public BaseMockProcess() {
this.in = new ByteArrayInputStream(new byte[0]);
this.out = new ByteArrayOutputStream();
}
@@ -113,6 +129,26 @@ public class LSPBindingsTest {
@Override
public boolean isAlive() {
+ return true;
+ }
+
+ @Override
+ public int exitValue() {
+ return 0;
+ }
+
+ @Override
+ public void destroy() {
+ }
+ }
+
+ static final class MockProcess extends BaseMockProcess {
+
+ public MockProcess() {
+ }
+
+ @Override
+ public boolean isAlive() {
StackTraceElement[] stack = new Exception().getStackTrace();
if (stack[1].getMethodName().equals("initServer")) {
return false;
@@ -120,13 +156,99 @@ public class LSPBindingsTest {
return true;
}
+ }
+
+ @Test
+ public void testNoContinuousRestarts1() throws Exception {
+ class MockLSP implements LanguageServerProvider {
+ @Override
+ public LanguageServerDescription startServer(Lookup lookup) {
+ final BaseMockProcess process = new BaseMockProcess() {
+ @Override
+ public boolean isAlive() {
+ return false;
+ }
+ };
+ return LanguageServerDescription.create(process.in, process.out, process);
+ }
+ }
+
+ MockServices.setServices(MockMimeLookupCacheSPI.class, MockMimeResolver.class);
+
+ MIME_LOOKUP.setLookup(Lookups.fixed(new MockLSP()));
+
+ FileObject folder = FileUtil.createMemoryFileSystem().getRoot().createFolder("myfolder");
+ FileObject file = folder.createData("data.mock-txt");
+ assertEquals("application/mock-txt", file.getMIMEType());
+ assertNull("No project owner", FileOwnerQuery.getOwner(file));
+
+ for (int i = 0; i < 5; i++) {
+ LSPBindings.getBindings(file);
+ }
+
+ NotifyDisplayerImpl.called.set(0);
+
+ LSPBindings bindings = LSPBindings.getBindings(file);
+ assertNull("No bindings after many failures", bindings);
+ assertEquals(1, NotifyDisplayerImpl.called.get());
+ }
+
+ @Test
+ public void testNoContinuousRestarts2() throws Exception {
+ class MockLSP implements LanguageServerProvider {
+ @Override
+ public LanguageServerDescription startServer(Lookup lookup) {
+ final BaseMockProcess process = new BaseMockProcess() {
+ @Override
+ public boolean isAlive() {
+ return false;
+ }
+ };
+ OutputStream closed = new OutputStream() {
+ @Override
+ public void write(int b) throws IOException {
+ throw new IOException();
+ }
+ };
+ return LanguageServerDescription.create(process.in, closed, process);
+ }
+ }
+
+ MockServices.setServices(MockMimeLookupCacheSPI.class, MockMimeResolver.class);
+
+ MIME_LOOKUP.setLookup(Lookups.fixed(new MockLSP()));
+
+ FileObject folder = FileUtil.createMemoryFileSystem().getRoot().createFolder("myfolder");
+ FileObject file = folder.createData("data.mock-txt");
+ assertEquals("application/mock-txt", file.getMIMEType());
+ assertNull("No project owner", FileOwnerQuery.getOwner(file));
+
+ for (int i = 0; i < 5; i++) {
+ LSPBindings.getBindings(file);
+ }
+
+ NotifyDisplayerImpl.called.set(0);
+
+ LSPBindings bindings = LSPBindings.getBindings(file);
+ assertNull("No bindings after many failures", bindings);
+ assertEquals(1, NotifyDisplayerImpl.called.get());
+ }
+
+ @ServiceProvider(service=NotificationDisplayer.class, position=100)
+ public static class NotifyDisplayerImpl extends NotificationDisplayer {
+
+ private static final AtomicInteger called = new AtomicInteger();
+
@Override
- public int exitValue() {
- return 0;
+ public Notification notify(String title, Icon icon, String detailsText, ActionListener detailsAction, Priority priority) {
+ called.incrementAndGet();
+ return null;
}
@Override
- public void destroy() {
+ public Notification notify(String title, Icon icon, JComponent balloonDetails, JComponent popupDetails, Priority priority) {
+ throw new UnsupportedOperationException("Not supported yet.");
}
+
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@netbeans.apache.org
For additional commands, e-mail: commits-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists