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

Fix a bunch of race conditions in SpringIndexer and its tests

Browse Source
This commit is contained in:
Kris De Volder
2018-03-27 11:39:24 -07:00
parent 502d04d5a3
commit 57fa91fe5c
3 changed files with 80 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
headless-services/commons/commons-java/src/main/java/org/springframework/ide/vscode/commons/languageserver/java/AbstractJavaProjectCache.java
View File

@@ -97,7 +97,7 @@ public abstract class AbstractJavaProjectCache<K, P extends IJavaProject> implem
@Override
public void addListener(Listener listener) {
log.info("Add listener {} to {}", listener, this);
log.debug("Add listener {} to {}", listener, this);
listeners.add(listener);
}
@@ -107,17 +107,17 @@ public abstract class AbstractJavaProjectCache<K, P extends IJavaProject> implem
}
final protected void notifyProjectCreated(P project) {
log.info("project created {}", project);
log.debug("project created {}", project);
listeners.forEach(l -> l.created(project));
}
final protected void notifyProjectChanged(P project) {
log.info("project changed {}", project);
log.debug("project changed {}", project);
listeners.forEach(l -> l.changed(project));
}
final protected void notifyProjectDeleted(P project) {
log.info("project deleted {}", project);
log.debug("project deleted {}", project);
listeners.forEach(l -> l.deleted(project));
}

83
headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringIndexer.java
View File

@@ -26,13 +26,15 @@ import java.util.concurrent.BlockingQueue;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import com.google.common.collect.ImmutableList;
import org.apache.commons.io.FileUtils;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.dom.AST;
@@ -67,13 +69,13 @@ import org.springframework.ide.vscode.commons.languageserver.java.ProjectObserve
import org.springframework.ide.vscode.commons.languageserver.java.ProjectObserver.Listener;
import org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer;
import org.springframework.ide.vscode.commons.languageserver.util.SimpleWorkspaceService;
import org.springframework.ide.vscode.commons.util.ExceptionUtil;
import org.springframework.ide.vscode.commons.util.Futures;
import org.springframework.ide.vscode.commons.util.StringUtil;
import org.springframework.ide.vscode.commons.util.UriUtil;
import org.springframework.ide.vscode.commons.util.text.LanguageId;
import org.springframework.ide.vscode.commons.util.text.TextDocument;
import com.google.common.collect.ImmutableList;
/**
* @author Martin Lippert
*/
@@ -180,23 +182,29 @@ public class SpringIndexer {
}
public CompletableFuture<Void> initialize(Collection<WorkspaceFolder> workspaceRoots) {
synchronized(this) {
try {
if (lastInitializeItem != null && !lastInitializeItem.getFuture().isDone()) {
log.debug("Canceling {}", lastInitializeItem);
lastInitializeItem.getFuture().cancel(false);
}
InitializeItem toCancel = null;
try {
synchronized(this) {
toCancel = lastInitializeItem;
//Careful do not cancel until created and setup new item. Otherwise it creates a
//race condition in the test harness which needs to be able to ensure initialization
//is completed.
lastInitializeItem = new InitializeItem(workspaceRoots.toArray(new WorkspaceFolder[workspaceRoots.size()]));
updateQueue.put(lastInitializeItem);
return lastInitializeItem.getFuture();
}
catch (Throwable e) {
log.error("{}", e);
} catch (Throwable e) {
log.error("", e);
return Futures.error(e);
} finally {
try {
if (toCancel!=null && !toCancel.getFuture().isDone()) {
toCancel.getFuture().cancel(false);
}
} catch (Exception e) {
//ignore
}
}
return null;
}
public boolean isInitializing() {
@@ -204,26 +212,26 @@ public class SpringIndexer {
}
public void waitForInitializeTask() {
synchronized (this) {
if (lastInitializeItem != null) {
InitializeItem lastInitializeItem = this.lastInitializeItem;
while (lastInitializeItem != null) {
if (!lastInitializeItem.getFuture().isDone()) {
try {
log.debug("Wating for {}", lastInitializeItem);
lastInitializeItem.getFuture().get();
} catch (InterruptedException | ExecutionException e) {
// ignore
} catch (Exception e) {
log.debug("Waiting for {} aborted", lastInitializeItem);
log.debug(ExceptionUtil.getMessage(e));
}
lastInitializeItem = this.lastInitializeItem;
} else {
log.debug("No need to wait for {}", lastInitializeItem);
lastInitializeItem = this.lastInitializeItem==lastInitializeItem ? null : this.lastInitializeItem;
}
}
}
private void refresh() {
synchronized (this) {
symbols.clear();
symbolsByDoc.clear();
addonInformation.clear();
addonInformationByDoc.clear();
Collection<WorkspaceFolder> roots = server.getWorkspaceRoots();
log.debug("refresh spring indexer for roots: {}", roots.toString());
initialize(roots);
@@ -276,11 +284,10 @@ public class SpringIndexer {
return deleteItem.getFuture();
}
catch (Exception e) {
log.error("{}", e);
log.error("", e);
return Futures.error(e);
}
}
return null;
}
public CompletableFuture<Void> createDocument(String docURI) {
@@ -298,12 +305,12 @@ public class SpringIndexer {
}
}
catch (Exception e) {
log.error("{}", e);
log.error("", e);
return Futures.error(e);
}
}
}
return null;
return CompletableFuture.completedFuture(null);
}
public List<SymbolInformation> getAllSymbols(String query) {
@@ -628,8 +635,12 @@ public class SpringIndexer {
}
private static AtomicInteger initItemId = new AtomicInteger(0);
private class InitializeItem implements WorkerItem {
private int id = initItemId.incrementAndGet();
private final WorkspaceFolder[] workspaceRoots;
private final CompletableFuture<Void> future;
@@ -650,7 +661,12 @@ public class SpringIndexer {
try {
if (!future.isCancelled()) {
// log.debug("initialze spring indexer task started for roots: " + Arrays.toString(workspaceRoots));
symbols.clear();
symbolsByDoc.clear();
addonInformation.clear();
addonInformationByDoc.clear();
for (WorkspaceFolder root : workspaceRoots) {
SpringIndexer.this.scanFiles(root);
}
@@ -667,6 +683,11 @@ public class SpringIndexer {
log.error("{} threw exception", this, e);
}
}
@Override
public String toString() {
return "InitItem("+id+")";
}
}
private class UpdateItem implements WorkerItem {

28
headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/SpringIndexerTest.java
View File

@@ -35,6 +35,7 @@ import org.springframework.ide.vscode.boot.java.requestmapping.RequestMappingSym
import org.springframework.ide.vscode.boot.java.utils.SpringIndexer;
import org.springframework.ide.vscode.commons.languageserver.composable.ComposableLanguageServer;
import org.springframework.ide.vscode.commons.maven.MavenCore;
import org.springframework.ide.vscode.commons.util.Assert;
import org.springframework.ide.vscode.languageserver.testharness.LanguageServerHarness;
import org.springframework.ide.vscode.project.harness.BootJavaLanguageServerHarness;
import org.springframework.ide.vscode.project.harness.ProjectsHarness;
@@ -139,9 +140,10 @@ public class SpringIndexerTest {
// update document and update index
String changedDocURI = directory.toPath().resolve("src/main/java/org/test/SimpleMappingClass.java").toUri().toString();
assertTrue(containsSymbol(indexer().getSymbols(changedDocURI), "@/mapping1", changedDocURI));
String newContent = FileUtils.readFileToString(new File(new URI(changedDocURI))).replace("mapping1", "mapping1-CHANGED");
CompletableFuture<Void> updateFuture = indexer().updateDocument(changedDocURI, newContent);
updateFuture.get(5, TimeUnit.SECONDS);
// check for updated index per document
@@ -167,6 +169,8 @@ public class SpringIndexerTest {
assertTrue(containsSymbol(allSymbols, "@/classlevel/mapping-subpackage", docUri, 7, 1, 7, 38));
}
@Test
public void testNewDocumentCreated() throws Exception {
harness.intialize(new File(ProjectsHarness.class.getResource("/test-projects/test-annotation-indexing-parent/test-annotation-indexing/").toURI()));
@@ -242,12 +246,13 @@ public class SpringIndexerTest {
// update document and update index
String deletedDocURI = directory.toPath().resolve("src/main/java/org/test/SimpleMappingClass.java").toUri().toString();
assertFalse(indexer().getSymbols(deletedDocURI).isEmpty()); //We have symbols before deletion?
CompletableFuture<Void> deleteFuture = indexer().deleteDocument(deletedDocURI);
deleteFuture.get(5, TimeUnit.SECONDS);
deleteFuture.get(5, TimeUnit.HOURS);
// check for updated index per document
List<? extends SymbolInformation> symbols = indexer().getSymbols(deletedDocURI);
assertNull(symbols);
Assert.noElements(indexer().getSymbols(deletedDocURI));
// check for updated index in all symbols
List<? extends SymbolInformation> allSymbols = indexer().getAllSymbols("");
@@ -314,6 +319,21 @@ public class SpringIndexerTest {
assertTrue(containsSymbol(allSymbols, "@/foo-root-mapping/embedded-foo-mapping-with-root", docUri, 27, 1, 27, 51));
}
private boolean containsSymbol(List<? extends SymbolInformation> symbols, String name, String uri) {
for (Iterator<? extends SymbolInformation> iterator = symbols.iterator(); iterator.hasNext();) {
SymbolInformation symbol = iterator.next();
if (
symbol.getName().equals(name) &&
symbol.getLocation().getUri().equals(uri)
) {
return true;
}
}
return false;
}
private boolean containsSymbol(List<? extends SymbolInformation> symbols, String name, String uri, int startLine, int startCHaracter, int endLine, int endCharacter) {
for (Iterator<? extends SymbolInformation> iterator = symbols.iterator(); iterator.hasNext();) {
SymbolInformation symbol = iterator.next();