Skip to content

Fine grain locking to avoid deadlocks in CU cache #1509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fine grain locking to avoid deadlocks in CU cache #1509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions ...er/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,6 @@ public <T> T withCompilationUnit(IJavaProject project, URI uri, Function<Compila
logger.info("CU Cache: work item submitted for doc {}", uri.toASCIIString());

if (project != null) {
ReadLock lock = environmentCacheLock.readLock();
lock.lock();
try {
CompilationUnit cu = null;
try {
Expand All @@ -234,10 +232,16 @@ public <T> T withCompilationUnit(IJavaProject project, URI uri, Function<Compila
}

if (cu != null) {
projectToDocs.get(project.getLocationUri(), () -> new HashSet<>()).add(uri);

logger.debug("CU Cache: start work on AST for {}", uri.toString());
ReadLock lock = environmentCacheLock.readLock();
lock.lock();
try {
logger.info("CU Cache: start work on AST for {}", uri.toString());
return requestor.apply(cu);
} finally {
logger.info("CU Cache: end work on AST for {}", uri.toString());
lock.unlock();
}

}
}
catch (CancellationException e) {
Expand All @@ -246,10 +250,6 @@ public <T> T withCompilationUnit(IJavaProject project, URI uri, Function<Compila
catch (Exception e) {
logger.error("", e);
}
finally {
logger.debug("CU Cache: end work on AST for {}", uri.toString());
lock.unlock();
}
}

return requestor.apply(null);
Expand All @@ -259,34 +259,49 @@ private synchronized CompletableFuture<CompilationUnit> requestCU(IJavaProject p
CompletableFuture<CompilationUnit> cuFuture = uriToCu.getIfPresent(uri);
if (cuFuture == null) {
cuFuture = CompletableFuture.supplyAsync(() -> {
ReadLock lock = environmentCacheLock.readLock();
lock.lock();
logger.info("Started parsing CU for " + uri);

try {
logger.info("Started parsing CU for " + uri);
Tuple2<List<Classpath>, INameEnvironmentWithProgress> lookupEnvTuple = loadLookupEnvTuple(project);
String uriStr = uri.toASCIIString();
String unitName = uriStr.substring(uriStr.lastIndexOf("/") + 1); // skip over '/'
CompilationUnit cUnit = parse2(fetchContent(uri).toCharArray(), uriStr, unitName, lookupEnvTuple.getT1(), lookupEnvTuple.getT2(),
annotationHierarchies.get(project.getLocationUri(), AnnotationHierarchies::new));

logger.debug("CU Cache: created new AST for {}", uri.toASCIIString());

logger.info("Parsed successfully CU for " + uri);
return cUnit;
} catch (Throwable t) {
// Complete future exceptionally
throw new CompletionException(t);
} finally {
logger.info("Finished parsing CU for {}", uri);
lock.unlock();
}
}, createCuExecutorThreadPool);
// Cache the future
uriToCu.put(uri, cuFuture);
// If CU future completed exceptionally invalidate the cache entry
cuFuture.exceptionally(t -> {
if (!(t instanceof CancellationException)) {
logger.error("", t);
}
uriToCu.invalidate(uri);
return null;
});
cuFuture
.thenAccept(cu -> {
synchronized(CompilationUnitCache.this) {
try {
projectToDocs.get(project.getLocationUri(), () -> new HashSet<>()).add(uri);
} catch (ExecutionException e) {
// shouldn't happen
}
}
})
.exceptionally(t -> {
if (!(t instanceof CancellationException)) {
logger.error("", t);
}
synchronized(CompilationUnitCache.this) {
uriToCu.invalidate(uri);
}
return null;
});
}
return cuFuture;
}
Expand Down
57 changes: 33 additions & 24 deletions ...ge-server/src/main/java/org/springframework/ide/vscode/boot/jdt/ls/JdtLsProjectCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,43 +338,52 @@ public void initialize(InitializeParams p, ServerCapabilities cap) {

private class JstLsClasspathListener implements ClasspathListener {

/*
* Synchronize to make non-reentrant such that events handled in predictable order
*/
@Override
public void changed(Event event) {
public synchronized void changed(Event event) {
log.debug("claspath event received {}", event);
server.doOnInitialized(() -> {
try {
synchronized (table) {
String uri = UriUtil.normalize(event.projectUri);
log.debug("uri = {}", uri);
if (event.deleted) {
log.debug("event.deleted = true");
IJavaProject deleted = table.remove(uri);
if (deleted!=null) {
log.debug("removed from table = true");
notifyDelete(deleted);
} else {
log.warn("Deleted project not removed because uri {} not found in {}", uri, table.keySet());
}
String uri = UriUtil.normalize(event.projectUri);
log.debug("uri = {}", uri);
if (event.deleted) {
log.debug("event.deleted = true");
IJavaProject deleted;
synchronized (table) {
deleted = table.remove(uri);
}
// Notify outside of the lock
if (deleted!=null) {
log.debug("removed from table = true");
notifyDelete(deleted);
} else {
log.debug("deleted = false");
URI projectUri = new URI(uri);
ClasspathData classpath = new ClasspathData(event.name, event.classpath.getEntries(), event.classpath.getJavaVersion());
IJavaProject oldProject = table.get(uri);
log.warn("Deleted project not removed because uri {} not found in {}", uri, table.keySet());
}
} else {
log.debug("deleted = false");
URI projectUri = new URI(uri);
ClasspathData classpath = new ClasspathData(event.name, event.classpath.getEntries(), event.classpath.getJavaVersion());
IJavaProject oldProject, newProject;
synchronized(table) {
oldProject = table.get(uri);
if (oldProject != null && classpath.equals(oldProject.getClasspath())) {
// nothing has changed
return;
}
IProjectBuild projectBuild = from(event.projectBuild);
IJavaProject newProject = IS_JANDEX_INDEX
IProjectBuild projectBuild = from(event.projectBuild);
newProject = IS_JANDEX_INDEX
? new JavaProject(getFileObserver(), projectUri, classpath,
JdtLsProjectCache.this, projectBuild)
: new JdtLsJavaProject(server.getClient(), projectUri, classpath, JdtLsProjectCache.this, projectBuild);
table.put(uri, newProject);
if (oldProject != null) {
notifyChanged(newProject);
} else {
notifyCreated(newProject);
}
}
// Notify outside of the lock
if (oldProject != null) {
notifyChanged(newProject);
} else {
notifyCreated(newProject);
}
}
} catch (Exception e) {
Expand Down