Spring/spring-tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Some refactorings towards fixing a nasty race condition

Browse Source 
See: https://www.pivotaltracker.com/story/show/143694789

This work is not yet complete (everything still works as before, but a race condtion between clearing AST cache and using AST for reconciles remains).
This commit is contained in:
Kris De Volder
2017-04-14 10:41:20 -07:00
parent c520ef833e
commit e0103c1352
13 changed files with 148 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/BootJavaLanguageServer.java
View File

@@ -54,7 +54,7 @@ public class BootJavaLanguageServer extends SimpleLanguageServer {
IReconcileEngine reconcileEngine = new BootJavaReconcileEngine();
documents.onDidChangeContent(params -> {
TextDocument doc = params.getDocument();
validateWith(doc, reconcileEngine);
validateWith(doc.getId(), reconcileEngine);
});
ICompletionEngine bootCompletionEngine = new BootJavaCompletionEngine(javaProjectFinder, indexProvider);

2
headless-services/boot-properties-language-server/src/main/java/org/springframework/ide/vscode/boot/BootPropertiesLanguageServer.java
View File

@@ -103,7 +103,7 @@ public class BootPropertiesLanguageServer extends SimpleLanguageServer {
IReconcileEngine reconcileEngine = getReconcileEngine();
documents.onDidChangeContent(params -> {
TextDocument doc = params.getDocument();
validateWith(doc, reconcileEngine);
validateWith(doc.getId(), reconcileEngine);
});
ICompletionEngine propertiesCompletionEngine = getCompletionEngine();

14
headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/LaunguageServerApp.java
View File

@@ -161,7 +161,7 @@ public abstract class LaunguageServerApp {
SimpleLanguageServer languageServer = createServer();
Launcher<STS4LanguageClient> launcher = createSocketLauncher(languageServer, STS4LanguageClient.class,
new InetSocketAddress("localhost", SERVER_STANDALONE_PORT), Executors.newCachedThreadPool(), wrapper);
new InetSocketAddress("localhost", SERVER_STANDALONE_PORT), createServerThreads(), wrapper);
languageServer.connect(launcher.getRemoteProxy());
Future<?> future = launcher.startListening();
@@ -170,7 +170,15 @@ public abstract class LaunguageServerApp {
}
}
private <T> Launcher<T> createSocketLauncher(Object localService, Class<T> remoteInterface, SocketAddress socketAddress, ExecutorService executorService, Function<MessageConsumer, MessageConsumer> wrapper) throws IOException {
/**
* Creates the thread pool / executor passed to lsp4j server intialization. From the looks of things,
* @return
*/
protected ExecutorService createServerThreads() {
return Executors.newSingleThreadExecutor();
}
private <T> Launcher<T> createSocketLauncher(Object localService, Class<T> remoteInterface, SocketAddress socketAddress, ExecutorService executorService, Function<MessageConsumer, MessageConsumer> wrapper) throws IOException {
AsynchronousServerSocketChannel serverSocket = AsynchronousServerSocketChannel.open().bind(socketAddress);
AsynchronousSocketChannel socketChannel;
try {
@@ -215,7 +223,7 @@ public abstract class LaunguageServerApp {
*/
protected void run(Connection connection) throws InterruptedException, ExecutionException {
LanguageServer server = createServer();
ExecutorService executor = Executors.newCachedThreadPool();
ExecutorService executor = createServerThreads();
Function<MessageConsumer, MessageConsumer> wrapper = (MessageConsumer consumer) -> {
return (msg) -> {
try {

5
headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/DocumentSymbolHandler.java
View File

@@ -11,7 +11,6 @@
package org.springframework.ide.vscode.commons.languageserver.util;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import org.eclipse.lsp4j.DocumentSymbolParams;
import org.eclipse.lsp4j.SymbolInformation;
@@ -21,8 +20,8 @@ import com.google.common.collect.ImmutableList;
@FunctionalInterface
public interface DocumentSymbolHandler {
DocumentSymbolHandler NO_SYMBOLS = (params) -> CompletableFuture.completedFuture(ImmutableList.of());
DocumentSymbolHandler NO_SYMBOLS = (params) -> ImmutableList.of();
CompletableFuture<List<? extends SymbolInformation>> handle(DocumentSymbolParams params);
List<? extends SymbolInformation> handle(DocumentSymbolParams params);
}

104
headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java
View File

@@ -27,6 +27,7 @@ import org.eclipse.lsp4j.MessageParams;
import org.eclipse.lsp4j.MessageType;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.ServerCapabilities;
import org.eclipse.lsp4j.TextDocumentIdentifier;
import org.eclipse.lsp4j.TextDocumentSyncKind;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;
@@ -46,6 +47,7 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemSe
import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem;
import org.springframework.ide.vscode.commons.util.BadLocationException;
import org.springframework.ide.vscode.commons.util.CollectionUtil;
import org.springframework.ide.vscode.commons.util.Log;
import org.springframework.ide.vscode.commons.util.text.TextDocument;
import reactor.core.publisher.Mono;
@@ -221,60 +223,67 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl
* Convenience method. Subclasses can call this to use a {@link IReconcileEngine} ported
* from old STS codebase to validate a given {@link TextDocument} and publish Diagnostics.
*/
protected void validateWith(TextDocument _doc, IReconcileEngine engine) {
TextDocument doc = _doc.copy();
protected void validateWith(TextDocumentIdentifier docId, IReconcileEngine engine) {
CompletableFuture<Void> reconcileSession = this.busyReconcile = new CompletableFuture<Void>();
Log.debug("Reconciling BUSY");
SimpleTextDocumentService documents = getTextDocumentService();
IProblemCollector problems = new IProblemCollector() {
private List<Diagnostic> diagnostics = new ArrayList<>();
private List<Quickfix> quickfixes = new ArrayList<>();
@Override
public void endCollecting() {
documents.setQuickfixes(doc, quickfixes);
documents.publishDiagnostics(doc, diagnostics);
}
@Override
public void beginCollecting() {
diagnostics.clear();
}
@Override
public void accept(ReconcileProblem problem) {
try {
DiagnosticSeverity severity = getDiagnosticSeverity(problem);
if (severity!=null) {
Diagnostic d = new Diagnostic();
d.setCode(problem.getCode());
d.setMessage(problem.getMessage());
Range rng = doc.toRange(problem.getOffset(), problem.getLength());
d.setRange(rng);
d.setSeverity(severity);
List<QuickfixData<?>> fixes = problem.getQuickfixes();
if (CollectionUtil.hasElements(fixes)) {
for (QuickfixData<?> fix : fixes) {
quickfixes.add(new Quickfix<>(EXTENSION_ID, rng, fix));
}
}
diagnostics.add(d);
}
} catch (BadLocationException e) {
LOG.log(Level.WARNING, "Invalid reconcile problem ignored", e);
}
}
};
// Avoid running in the same thread as lsp4j as it can result
// in long "hangs" for slow reconcile providers
Mono.fromRunnable(() -> {
TextDocument doc = documents.getDocument(docId.getUri()).copy();
IProblemCollector problems = new IProblemCollector() {
private List<Diagnostic> diagnostics = new ArrayList<>();
private List<Quickfix> quickfixes = new ArrayList<>();
@Override
public void endCollecting() {
documents.setQuickfixes(docId, quickfixes);
documents.publishDiagnostics(docId, diagnostics);
}
@Override
public void beginCollecting() {
diagnostics.clear();
}
@Override
public void accept(ReconcileProblem problem) {
try {
DiagnosticSeverity severity = getDiagnosticSeverity(problem);
if (severity!=null) {
Diagnostic d = new Diagnostic();
d.setCode(problem.getCode());
d.setMessage(problem.getMessage());
Range rng = doc.toRange(problem.getOffset(), problem.getLength());
d.setRange(rng);
d.setSeverity(severity);
List<QuickfixData<?>> fixes = problem.getQuickfixes();
if (CollectionUtil.hasElements(fixes)) {
for (QuickfixData<?> fix : fixes) {
quickfixes.add(new Quickfix<>(EXTENSION_ID, rng, fix));
}
}
diagnostics.add(d);
}
} catch (BadLocationException e) {
LOG.log(Level.WARNING, "Invalid reconcile problem ignored", e);
}
}
};
// try {
// Thread.sleep(2000);
// } catch (InterruptedException e) {
// }
Log.debug("Reconciling: "+doc);
engine.reconcile(doc, problems);
})
.doOnTerminate((ignore1, ignore2) -> {
reconcileSession.complete(null);
Log.debug("Reconciler DONE : "+this.busyReconcile.isDone());
})
.subscribeOn(RECONCILER_SCHEDULER)
.subscribe();
@@ -295,13 +304,8 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl
}
/**
* If reconciler is in progress, waits for it.
* <p>
* WARNING: this is quick and dirty hack, its good enough for test harness, if used
* with care, but probably not good enough to avoid all race conditions caused by
* stuff using old infos cached by the reconciler after underlying document has changed
* (because there could be a short delay between the moment the reconcile becomes 'busy' and the moment
* when the document change was processed.
* If reconciling is in progress, waits until reconciling has caught up to
* all the document changes.
*/
public void waitForReconcile() throws Exception {
while (!this.busyReconcile.isDone()) {

69
headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java
View File

@@ -18,8 +18,6 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.eclipse.lsp4j.CodeActionParams;
@@ -47,6 +45,8 @@ import org.eclipse.lsp4j.RenameParams;
import org.eclipse.lsp4j.SignatureHelp;
import org.eclipse.lsp4j.SymbolInformation;
import org.eclipse.lsp4j.TextDocumentContentChangeEvent;
import org.eclipse.lsp4j.TextDocumentIdentifier;
import org.eclipse.lsp4j.TextDocumentItem;
import org.eclipse.lsp4j.TextDocumentPositionParams;
import org.eclipse.lsp4j.TextEdit;
import org.eclipse.lsp4j.VersionedTextDocumentIdentifier;
@@ -58,17 +58,16 @@ import org.springframework.ide.vscode.commons.languageserver.LanguageIds;
import org.springframework.ide.vscode.commons.languageserver.quickfix.Quickfix;
import org.springframework.ide.vscode.commons.util.Assert;
import org.springframework.ide.vscode.commons.util.BadLocationException;
import org.springframework.ide.vscode.commons.util.ExceptionUtil;
import org.springframework.ide.vscode.commons.util.Log;
import org.springframework.ide.vscode.commons.util.text.TextDocument;
import com.google.common.collect.ImmutableList;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
public class SimpleTextDocumentService implements TextDocumentService {
private static final Logger LOG = Logger.getLogger(SimpleTextDocumentService.class.getName());
final private SimpleLanguageServer server;
private Map<String, TrackedDocument> documents = new HashMap<>();
private ListenerList<TextDocumentContentChange> documentChangeListeners = new ListenerList<>();
@@ -130,40 +129,29 @@ public class SimpleTextDocumentService implements TextDocumentService {
try {
VersionedTextDocumentIdentifier docId = params.getTextDocument();
String url = docId.getUri();
//LOG.info("didChange: "+url);
Log.debug("didChange: "+url);
if (url!=null) {
TextDocument doc = getDocument(url);
for (TextDocumentContentChangeEvent change : params.getContentChanges()) {
doc.apply(change);
didChangeContent(doc, change);
}
doc.setVersion(docId.getVersion());
}
} catch (BadLocationException e) {
LOG.log(Level.SEVERE, ExceptionUtil.getMessage(e), e);
Log.log(e);
}
}
@Override
public void didOpen(DidOpenTextDocumentParams params) {
//LOG.info("didOpen: "+params.getUri());
//Example message:
//{
// "jsonrpc":"2.0",
// "method":"textDocument/didOpen",
// "params":{
// "textDocument":{
// "uri":"file:///home/kdvolder/tmp/hello-java/hello.txt",
// "languageId":"plaintext",
// "version":1,
// "text":"This is some text ya-all o\nsss typescript\n"
// }
// }
//}
String url = params.getTextDocument().getUri();
String languageId = params.getTextDocument().getLanguageId();
TextDocumentItem docId = params.getTextDocument();
String url = docId.getUri();
String languageId = docId.getLanguageId();
int version = docId.getVersion();
if (url!=null) {
String text = params.getTextDocument().getText();
TextDocument doc = createDocument(url, languageId).getDocument();
TextDocument doc = createDocument(url, languageId, version).getDocument();
doc.setText(text);
TextDocumentContentChangeEvent change = new TextDocumentContentChangeEvent() {
@Override
@@ -206,17 +194,17 @@ public class SimpleTextDocumentService implements TextDocumentService {
public synchronized TextDocument getDocument(String url) {
TrackedDocument doc = documents.get(url);
if (doc==null) {
LOG.warning("Trying to get document ["+url+"] but it did not exists. Creating it with language-id 'plaintext'");
doc = createDocument(url, LanguageIds.PLAINTEXT);
Log.warn("Trying to get document ["+url+"] but it did not exists. Creating it with language-id 'plaintext'");
doc = createDocument(url, LanguageIds.PLAINTEXT, 0);
}
return doc.getDocument();
}
private synchronized TrackedDocument createDocument(String url, String languageId) {
private synchronized TrackedDocument createDocument(String url, String languageId, int version) {
if (documents.get(url)!=null) {
LOG.warning("Creating document ["+url+"] but it already exists. Existing document discarded!");
Log.warn("Creating document ["+url+"] but it already exists. Existing document discarded!");
}
TrackedDocument doc = new TrackedDocument(new TextDocument(url, languageId));
TrackedDocument doc = new TrackedDocument(new TextDocument(url, languageId, version));
documents.put(url, doc);
return doc;
}
@@ -280,10 +268,18 @@ public class SimpleTextDocumentService implements TextDocumentService {
@Override
public CompletableFuture<List<? extends SymbolInformation>> documentSymbol(DocumentSymbolParams params) {
if (documentSymbolHandler!=null) {
return documentSymbolHandler.handle(params);
DocumentSymbolHandler documentSymbolHandler = this.documentSymbolHandler;
if (documentSymbolHandler==null) {
return CompletableFuture.completedFuture(ImmutableList.of());
}
return CompletableFuture.completedFuture(Collections.emptyList());
return Mono.fromCallable(() -> {
Log.debug("documentSymbol request waiting for reconcile: "+params.getTextDocument());
server.waitForReconcile();
Log.info("documentSymbol request proceeding: "+params.getTextDocument());
return documentSymbolHandler.handle(params);
})
.toFuture()
.thenApply(l -> (List<? extends SymbolInformation>)l);
}
@Override
@@ -335,18 +331,19 @@ public class SimpleTextDocumentService implements TextDocumentService {
public void didSave(DidSaveTextDocumentParams params) {
}
public void publishDiagnostics(TextDocument doc, List<Diagnostic> diagnostics) {
public void publishDiagnostics(TextDocumentIdentifier docId, List<Diagnostic> diagnostics) {
LanguageClient client = server.getClient();
if (client!=null && diagnostics!=null) {
PublishDiagnosticsParams params = new PublishDiagnosticsParams();
params.setUri(doc.getUri());
params.setUri(docId.getUri());
params.setDiagnostics(diagnostics);
client.publishDiagnostics(params);
//Log.info("publishDiagnostics: "+params);
}
}
public void setQuickfixes(TextDocument doc, List<Quickfix> quickfixes) {
TrackedDocument td = documents.get(doc.getUri());
public void setQuickfixes(TextDocumentIdentifier docId, List<Quickfix> quickfixes) {
TrackedDocument td = documents.get(docId.getUri());
if (td!=null) {
td.setQuickfixes(quickfixes);
}

15
headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Log.java
View File

@@ -34,4 +34,19 @@ public class Log {
logger.error(message);
}
public static void info(String info) {
logger.info(info);
}
public static void warn(String string) {
logger.warn(string);
}
/**
* Note: to enable debug output set this in launchconfig: -Dorg.slf4j.simpleLogger.log.org.springframework.ide.vscode.commons.util.Log=debug
*/
public static void debug(String string) {
logger.debug(string);
}
}

23
headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/text/TextDocument.java
View File

@@ -17,6 +17,7 @@ import java.util.regex.Pattern;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.TextDocumentContentChangeEvent;
import org.eclipse.lsp4j.TextDocumentIdentifier;
import org.springframework.ide.vscode.commons.util.BadLocationException;
import org.springframework.ide.vscode.commons.util.text.linetracker.DefaultLineTracker;
import org.springframework.ide.vscode.commons.util.text.linetracker.ILineTracker;
@@ -31,10 +32,10 @@ public class TextDocument implements IDocument {
private final String languageId;
private final String uri;
private Text text = new Text("");
private int version;
public TextDocument(String uri, String languageId) {
this.uri = uri;
this.languageId = languageId;
this(uri, languageId, 0);
}
private TextDocument(TextDocument other) {
@@ -42,6 +43,13 @@ public class TextDocument implements IDocument {
this.languageId = other.getLanguageId();
this.text = other.text;
this.lineTracker.set(text.toString());
this.version = other.version;
}
public TextDocument(String uri, String languageId, int version) {
this.uri = uri;
this.languageId = languageId;
this.version = version;
}
@Override
@@ -261,4 +269,15 @@ public class TextDocument implements IDocument {
return toRange(region.getOffset(), region.getLength());
}
public void setVersion(int version) {
this.version = version;
}
public TextDocumentIdentifier getId() {
if (uri!=null) {
return new TextDocumentIdentifier(uri);
}
return null;
}
}

5
headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseDocumentSymbolHandler.java
View File

@@ -14,7 +14,6 @@ import java.util.Collection;
import java.util.List;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import org.eclipse.lsp4j.DocumentSymbolParams;
import org.eclipse.lsp4j.Location;
@@ -59,7 +58,7 @@ public class ConcourseDocumentSymbolHandler implements DocumentSymbolHandler {
}
@Override
public CompletableFuture<List<? extends SymbolInformation>> handle(DocumentSymbolParams params) {
public List<? extends SymbolInformation> handle(DocumentSymbolParams params) {
Builder<SymbolInformation> builder = ImmutableList.builder();
TextDocument doc = documents.getDocument(params.getTextDocument().getUri());
for (Entry<Node, YType> entry : astTypeCache.getNodes(params.getTextDocument().getUri()).entrySet()) {
@@ -71,7 +70,7 @@ public class ConcourseDocumentSymbolHandler implements DocumentSymbolHandler {
}
}
}
return CompletableFuture.completedFuture(builder.build());
return builder.build();
}
protected SymbolInformation createSymbol(TextDocument doc, Node node, YType type) throws BadLocationException {

6
headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java
View File

@@ -91,11 +91,11 @@ public class ConcourseLanguageServer extends SimpleLanguageServer {
documents.onDidChangeContent(params -> {
TextDocument doc = params.getDocument();
if (LanguageIds.CONCOURSE_PIPELINE.equals(doc.getLanguageId())) {
validateWith(doc, forPipelines.reconcileEngine);
validateWith(doc.getId(), forPipelines.reconcileEngine);
} else if (LanguageIds.CONCOURSE_TASK.equals(doc.getLanguageId())) {
validateWith(doc, forTasks.reconcileEngine);
validateWith(doc.getId(), forTasks.reconcileEngine);
} else {
validateWith(doc, IReconcileEngine.NULL);
validateWith(doc.getId(), IReconcileEngine.NULL);
}
});

1
headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java
View File

@@ -131,6 +131,7 @@ public class ConcourseModel {
private void documentChanged(TextDocumentContentChange changeEvent) {
String uri = changeEvent.getDocument().getUri();
if (uri!=null) {
Log.debug("Clear AST cache: "+uri);
asts.invalidate(uri);
}
}

9
headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/util/StaleFallbackCache.java
View File

@@ -23,17 +23,14 @@ import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
/**
* Simple cache implementation that falls back on 'stale' cache entry if
* a new entry can not be computed. The api is loosely modeled after
* guava's Cache interface (but only the subset we use is implemented to reduce the
* complexity of its implementation).
* A simple cache implementation that provides an option for lookups to fallback
* to a 'stale' cache entry when computing a current one fails.
*/
public class StaleFallbackCache<K, V>{
Map<K, V> staleEntries = new HashMap<>();
Cache<K, CompletableFuture<V>> validEntries = CacheBuilder.newBuilder().build();
public synchronized V get(K key, boolean allowStaleEntries, Callable<? extends V> valueLoader) throws Exception {
CompletableFuture<V> valid = validEntries.get(key, () -> load(valueLoader));
if (!allowStaleEntries) {
@@ -48,7 +45,7 @@ public class StaleFallbackCache<K, V>{
return future_get(valid);
}
}
public synchronized void invalidate(K key) {
CompletableFuture<V> staleEntry = validEntries.getIfPresent(key);
if (staleEntry!=null) {

2
headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlLanguageServer.java
View File

@@ -75,7 +75,7 @@ public class ManifestYamlLanguageServer extends SimpleLanguageServer {
// SimpleWorkspaceService workspace = getWorkspaceService();
documents.onDidChangeContent(params -> {
TextDocument doc = params.getDocument();
validateWith(doc, engine);
validateWith(doc.getId(), engine);
});
// workspace.onDidChangeConfiguraton(settings -> {