Implement basic lock-free capability registration.

Author: toinehartman

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java

@@ -993,7 +993,7 @@ public synchronized void registerLanguage(LanguageParameter lang) {
         multiplexer.addContributor(buildContributionKey(lang),
             new InterpretedLanguageContributions(lang, this, availableWorkspaceService(), (IBaseLanguageClient)clientCopy, exec));
 
-        availableCapabilities().updateCapabilities(Collections.unmodifiableCollection(contributions.values()));
+        availableCapabilities().update(Collections.unmodifiableCollection(contributions.values()));
 
         fact.reloadContributions();
         fact.setClient(clientCopy);
@@ -1058,7 +1058,7 @@ public synchronized void unregisterLanguage(LanguageParameter lang) {
             contributions.remove(lang.getName());
         }
 
-        availableCapabilities().updateCapabilities(Collections.unmodifiableCollection(contributions.values()));
+        availableCapabilities().update(Collections.unmodifiableCollection(contributions.values()));
     }
 
     @Override

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/AbstractDynamicCapability.java

@@ -136,7 +136,7 @@ protected final boolean preferStaticRegistration() {
      *
      * If this capability prefers static registration or if the client does not support dynamic registration, set it statically.
      * @param client Client capabilities to determine dynamic registration support.
-     * @param result Server capabilities to modify when registerting statically.
+     * @param result Server capabilities to modify when registering statically.
      * @return `true` if this capability should be registered statically, `false` otherwise.
      */
     protected final boolean shouldRegisterStatically(ClientCapabilities client) {

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

@@ -36,6 +36,8 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
@@ -53,58 +55,59 @@
 import org.eclipse.lsp4j.services.LanguageClient;
 import org.rascalmpl.vscode.lsp.parametric.ILanguageContributions;
 import org.rascalmpl.vscode.lsp.util.Lists;
-import org.rascalmpl.vscode.lsp.util.NamedThreadPool;
 import org.rascalmpl.vscode.lsp.util.concurrent.CompletableFutureUtils;
 
 /**
  * Takes care of (un)registering capabilities dynamically.
- * Receives a list of supported capabiltities ({@link AbstractDynamicCapability}) and notifies the client when capabilities change.
+ * Receives a list of supported capabilities ({@link AbstractDynamicCapability}) and notifies the client when capabilities change.
  */
 public class DynamicCapabilities {
 
     private static final Logger logger = LogManager.getLogger(DynamicCapabilities.class);
 
     private final LanguageClient client;
     private final Executor exec;
-    private final Executor singleExec = NamedThreadPool.singleDaemon("parametric-capabilities");
-    private final CompletableFuture<Boolean> truthy;
-    private final CompletableFuture<Boolean> falsy;
-    private final Collection<AbstractDynamicCapability<?>> supportedCapabilities;
+    private final CompletableFuture<Void> noop;
 
-    // Set of capabilities that should bre registered statically instead of dynamically
+    private final Set<AbstractDynamicCapability<?>> dynamicCapabilities;
     private final Set<AbstractDynamicCapability<?>> staticCapabilities;
 
     // Map of method names with current registration values
+    private final AtomicReference<Collection<ILanguageContributions>> lastContributions = new AtomicReference<>(Collections.emptyList());
     private final Map<String, Registration> currentRegistrations = new ConcurrentHashMap<>();
 
     /**
      * @param client The language client to send register/unregister requests to.
      * @param supportedCapabilities The capabilities to register with the client.
      * @param clientCapabilities The capabilities of the client. Determine whether dynamic registration is supported at all.
      */
-    public DynamicCapabilities(LanguageClient client, Executor exec, List<AbstractDynamicCapability<?>> supportedCapabilities, ClientCapabilities clientCapabilities) {
+    public DynamicCapabilities(LanguageClient client, Executor exec, /* TODO -> Set */ List<AbstractDynamicCapability<?>> supportedCapabilities, ClientCapabilities clientCapabilities) {
         this.client = client;
         this.exec = exec;
-        this.truthy = CompletableFutureUtils.completedFuture(true, singleExec);
-        this.falsy = CompletableFutureUtils.completedFuture(false, singleExec);
-        this.supportedCapabilities = List.copyOf(supportedCapabilities);
+        this.noop = CompletableFutureUtils.completedFuture(null, exec);
 
-        // Check which capabilities to register statically
-        var caps = new HashSet<AbstractDynamicCapability<?>>();
+        // Check whether to register capabilities dynamically or statically
+        var dynamicCaps = new HashSet<AbstractDynamicCapability<?>>();
+        var staticCaps = new HashSet<AbstractDynamicCapability<?>>();
         for (var cap : supportedCapabilities) {
             if (cap.shouldRegisterStatically(clientCapabilities)) {
-                caps.add(cap);
+                logger.trace("Skipping capability {} for dynamic registration", cap.methodName());
+                staticCaps.add(cap);
+            } else {
+                dynamicCaps.add(cap);
             }
         }
         // Set once and only read from now on
-        this.staticCapabilities = Collections.unmodifiableSet(caps);
+        this.dynamicCapabilities = Collections.unmodifiableSet(dynamicCaps);
+        this.staticCapabilities = Collections.unmodifiableSet(staticCaps);
     }
 
     /**
      * Register static capabilities with the server.
      */
     public void registerStaticCapabilities(ServerCapabilities result) {
         for (var cap : staticCapabilities) {
+            logger.trace("Registering capability {} statically", cap.methodName());
             cap.registerStatically(result);
         }
     }
@@ -114,15 +117,14 @@ public void registerStaticCapabilities(ServerCapabilities result) {
      * @param contribs The contributions to represent.
      * @return A future that completes with a boolean that is false when any registration failed, and true otherwise.
      */
-    public CompletableFuture<Boolean> updateCapabilities(Collection<ILanguageContributions> contribs) {
-        // Copy the contributions so we know we are looking at a stable set of elements.
-        // If the contributions change, we expect our caller to call again.
-        var stableContribs = List.copyOf(contribs);
-        var registrations = supportedCapabilities.stream()
-            .filter(cap -> !staticCapabilities.contains(cap))
-            .map(c -> tryBuildRegistration(c, stableContribs)
-                .thenComposeAsync(r -> updateRegistration(c, r), singleExec));
-        return CompletableFutureUtils.reduce(registrations, CompletableFutureUtils.completedFuture(true, singleExec), Boolean::booleanValue, Boolean::logicalAnd);
+    public CompletableFuture<Void> update(Collection<ILanguageContributions> contribs) {
+        return CompletableFuture.supplyAsync(() -> {
+                logger.debug("Updating {} dynamic capabilities from {} contributions", dynamicCapabilities.size(), contribs.size());
+                // Copy the contributions so we know we are looking at a stable set of elements.
+                return lastContributions.updateAndGet(_old -> Set.copyOf(contribs));
+            }, exec)
+            .thenCompose(stableContribs -> CompletableFutureUtils.reduce(dynamicCapabilities.stream().map(cap -> updateRegistration(cap, stableContribs)), exec))
+            .thenAccept(_v -> logger.debug("Done updating dynamic capabilities"));
     }
 
     /**
@@ -134,31 +136,57 @@ public CompletableFuture<Boolean> updateCapabilities(Collection<ILanguageContrib
      * @param registration The computed registration to do, or `null` when this capability is absent.
      * @return A future completing with `true` when successful, or `false` otherwise.
      */
-    private <T> CompletableFuture<Boolean> updateRegistration(AbstractDynamicCapability<T> cap, @Nullable Registration registration) {
+    private <T> CompletableFuture<Void> updateRegistration(AbstractDynamicCapability<T> cap, Collection<ILanguageContributions> contribs) {
+        var reg = tryBuildRegistration(cap, contribs);
         var method = cap.methodName();
         var existingRegistration = currentRegistrations.get(method);
 
-        if (registration == null) {
-            if (existingRegistration != null) {
-                // this capability was removed
-                logger.trace("{} is no longer supported by contributions", method);
-                return unregister(existingRegistration);
+        return reg.<Void>thenCompose(registration -> {
+            Function<Boolean, CompletableFuture<Void>> checkDone = successful -> successful
+                ? eventuallyConsistent(cap, registration, contribs)
+                : noop;
+
+            if (registration == null) {
+                if (existingRegistration != null) {
+                    // this capability was removed
+                    logger.trace("{} is no longer supported by contributions", method);
+                    return unregister(existingRegistration)
+                        .thenCompose(checkDone);
+                }
+                logger.trace("{} is still not supported by contributions", method);
+                // nothing more to do
+                return checkDone.apply(true);
             }
-            // nothing more to do
-            return truthy;
-        }
 
-        if (existingRegistration != null) {
-            if (Objects.deepEquals(registration.getRegisterOptions(), existingRegistration.getRegisterOptions())) {
-                logger.trace("Options for {} did not change since last registration: {}", method, registration.getRegisterOptions());
-                return truthy;
+            if (existingRegistration != null) {
+                if (Objects.deepEquals(registration.getRegisterOptions(), existingRegistration.getRegisterOptions())) {
+                    logger.trace("Options for {} did not change since last registration", method);
+                    return checkDone.apply(true);
+                }
+                logger.trace("Options for {} changed since the previous registration ({} vs. {})", method, registration.getRegisterOptions(), existingRegistration.getRegisterOptions());
+                return register(registration, existingRegistration)
+                    .thenCompose(checkDone);
             }
-            logger.trace("Options for {} changed since the previous registration; remove before adding again", method);
-            return changeOptions(registration, existingRegistration);
+
+            logger.trace("Registering dynamic capability {}", registration);
+            return register(registration, existingRegistration)
+                .thenCompose(checkDone);
+        });
+    }
+
+    private <T> CompletableFuture<Void> eventuallyConsistent(AbstractDynamicCapability<T> cap, @Nullable Registration r, Collection<ILanguageContributions> contribs) {
+        var currentContribs = lastContributions.get();
+        var currentReg = currentRegistrations.get(cap.methodName());
+        if (contribs == currentContribs // instance comparison is safe since we took a read-only copy
+            && Objects.equals(currentReg, r)) {
+            // Nothing changed; we're done
+            return noop;
         }
 
-        logger.trace("Registering dynamic capability {}", method);
-        return register(registration);
+        // Something changed since we started...
+        // To be sure we arrive at a fixpoint, we need to go again
+        logger.trace("Something changed while registering {}; iterate until nothing changes", cap.methodName());
+        return updateRegistration(cap, currentContribs);
     }
 
     /**
@@ -168,20 +196,14 @@ private <T> CompletableFuture<Boolean> updateRegistration(AbstractDynamicCapabil
      * @return A future completing with `true` if successful, and `false` otherwise.
      */
     private CompletableFuture<Boolean> unregister(Registration reg) {
-        // If our administration contains exactly this registration, remove it and inform the client
-        if (!currentRegistrations.remove(reg.getMethod(), reg)) {
-            return falsy;
-        }
-
         return client.unregisterCapability(new UnregistrationParams(List.of(new Unregistration(reg.getId(), reg.getMethod()))))
             .handle((_v, t) -> {
-                if (t == null) {
-                    return true;
+                if (t != null) {
+                    handleError(t, "unregistering", reg.getMethod());
+                    return false;
                 }
-                handleError(t, "unregistering", reg.getMethod());
-                // Unregistration failed; put this back in our administration
-                currentRegistrations.putIfAbsent(reg.getMethod(), reg);
-                return false;
+                currentRegistrations.remove(reg.getMethod(), reg);
+                return true;
             });
     }
 
@@ -191,21 +213,15 @@ private CompletableFuture<Boolean> unregister(Registration reg) {
      * @param reg The registration to do.
      * @return A future completing with `true` if successful, and `false` otherwise.
      */
-    private CompletableFuture<Boolean> register(Registration reg) {
-        // If our administration contains no registration, inform the client
-        if (currentRegistrations.putIfAbsent(reg.getMethod(), reg) != null) {
-            return falsy;
-        }
-
+    private CompletableFuture<Boolean> register(Registration reg, @Nullable Registration existingRegistration) {
         return client.registerCapability(new RegistrationParams(List.of(reg)))
             .handle((_v, t) -> {
-                if (t == null) {
-                    return true;
+                if (t != null) {
+                    handleError(t, "registering", reg.getMethod());
+                    return false;
                 }
-                handleError(t, "registering", reg.getMethod());
-                // Registration failed; remove this from our administration
-                currentRegistrations.remove(reg.getMethod(), reg);
-                return false;
+                currentRegistrations.compute(reg.getMethod(), (k, v) -> Objects.equals(v, existingRegistration) ? reg : v);
+                return true;
             });
     }
 
@@ -216,27 +232,6 @@ private void handleError(Throwable t, String task, String method) {
         logger.error("Exception while {} capability {}", task, method, t);
     }
 
-    /**
-     * Update a registration.
-     * Aims to be atomic, i.e. keeps local administration of registered capabilities in sync with the client.
-     * @param newRegistration The registration with the updated options.
-     * @param existingRegistration The registration that we expect to currently be in place.
-     * @return A future completing with `true` if successful, or `false` otherwise.
-     */
-    private CompletableFuture<Boolean> changeOptions(Registration newRegistration, Registration existingRegistration) {
-        return unregister(existingRegistration)
-            .thenCompose(b -> {
-                if (!b.booleanValue()) {
-                    /* If unregistration fails, this has one of multiple causes:
-                        1. Someone raced us, won, and updated `currentRegistrations`. Our view of the current state is outdated, so we don't do anything and leave it to the winner.
-                        2. The unregistration request failed. This happens when a capability is not supported by the client. Since we successfully registered, this should not happen.
-                    */
-                    return falsy;
-                }
-                return register(newRegistration);
-            });
-    }
-
     private <T> CompletableFuture<@Nullable Registration> tryBuildRegistration(AbstractDynamicCapability<T> cap, Collection<ILanguageContributions> contribs) {
         if (contribs.isEmpty()) {
             return CompletableFutureUtils.completedFuture(null, exec);