Fine grain locking to avoid deadlocks in CU cache

Author: BoykoAlex

headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java

@@ -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 {
@@ -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) {
@@ -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);
@@ -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;
 	}

headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/jdt/ls/JdtLsProjectCache.java

@@ -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) {