[infinispan#14924] GLOBAL components should not start caches in their start method

Author: wburns

cdi/embedded/src/test/java/org/infinispan/cdi/embedded/test/cachemanager/external/Config.java

@@ -43,7 +43,7 @@ public class Config {
    @ApplicationScoped
    @SuppressWarnings("unused")
    public EmbeddedCacheManager defaultCacheManager() {
-      EmbeddedCacheManager externalCacheContainerManager = TestCacheManagerFactory.createCacheManager(false);
+      EmbeddedCacheManager externalCacheContainerManager = TestCacheManagerFactory.createCacheManager(true);
 
       // define large configuration
       externalCacheContainerManager.defineConfiguration("large", new ConfigurationBuilder()

core/src/main/java/org/infinispan/CoreModule.java

@@ -1,11 +1,43 @@
 package org.infinispan;
 
+import org.infinispan.commons.api.Lifecycle;
+import org.infinispan.factories.GlobalComponentRegistry;
 import org.infinispan.factories.annotations.InfinispanModule;
+import org.infinispan.globalstate.GlobalConfigurationManager;
 import org.infinispan.lifecycle.ModuleLifecycle;
+import org.infinispan.security.PrincipalRoleMapper;
+import org.infinispan.security.RolePermissionMapper;
 
 /**
  * @api.private
  */
 @InfinispanModule(name = "core")
 public class CoreModule implements ModuleLifecycle {
+   @Override
+   public void cacheManagerStarted(GlobalComponentRegistry gcr) {
+      gcr.getComponent(GlobalConfigurationManager.class).postStart();
+      startLifecycleComponent(gcr, RolePermissionMapper.class, PrincipalRoleMapper.class);
+   }
+
+   @Override
+   public void cacheManagerStopping(GlobalComponentRegistry gcr) {
+      stopLifecycleComponent(gcr, RolePermissionMapper.class, PrincipalRoleMapper.class);
+   }
+
+   public static void startLifecycleComponent(GlobalComponentRegistry gcr, Class<?>... klasses) {
+      for (Class<?> klass : klasses) {
+         if (gcr.getComponent(klass) instanceof Lifecycle l) {
+            l.start();
+         }
+      }
+   }
+
+   public static void stopLifecycleComponent(GlobalComponentRegistry gcr, Class<?>... klasses) {
+      for (Class<?> klass : klasses) {
+         if (gcr.getComponent(klass) instanceof Lifecycle l) {
+            l.stop();
+         }
+      }
+   }
+
 }

core/src/main/java/org/infinispan/cache/impl/CacheImpl.java

@@ -1036,6 +1036,7 @@ boolean lock(Collection<? extends K> keys, long flagsBitSet) {
    )
    public void start() {
       componentRegistry.start();
+      componentRegistry.postStart();
       queryProducer = componentRegistry.getComponent(QueryProducer.class);
 
       if (stateTransferManager != null) {

core/src/main/java/org/infinispan/cache/impl/SimpleCacheImpl.java

@@ -158,6 +158,7 @@ public void start() {
             .lifespan(configuration.expiration().lifespan())
             .maxIdle(configuration.expiration().maxIdle()).build();
       componentRegistry.start();
+      componentRegistry.postStart();
    }
 
    @Override

core/src/main/java/org/infinispan/factories/AbstractComponentRegistry.java

@@ -210,6 +210,9 @@ public void rewire() {
    /**
     * This starts the components in the registry, connecting to channels, starting service threads, etc.  If the component is
     * not in the {@link org.infinispan.lifecycle.ComponentStatus#INITIALIZING} state, it will be initialized first.
+    * <p>
+    * You need to call {@link #postStart()} after this completes with no error to complete startup. This is necessary
+    * as some post start steps can fail and this allows the user to handle that case before stopping the registry.
     */
    @Override
    public void start() {
@@ -235,14 +238,12 @@ public void start() {
          CompletionStage<Void> cs = delayStart();
          if (cs == null || CompletionStages.isCompletedSuccessfully(cs)) {
             updateStatusRunning();
-            postStart();
          } else {
             cs.whenComplete((ignore, t) -> {
                if (t != null) {
                   componentFailed(t);
                } else {
                   updateStatusRunning();
-                  postStart();
                }
             });
          }
@@ -258,7 +259,7 @@ private synchronized void updateStatusRunning() {
       }
    }
 
-   private void componentFailed(Throwable t) {
+   public void componentFailed(Throwable t) {
       synchronized (this) {
          state = ComponentStatus.FAILED;
          notifyAll();
@@ -278,7 +279,7 @@ private void componentFailed(Throwable t) {
 
    protected abstract void preStart();
 
-   protected abstract void postStart();
+   public abstract void postStart();
 
    abstract protected CompletionStage<Void> delayStart();
 

core/src/main/java/org/infinispan/factories/ComponentRegistry.java

@@ -258,7 +258,7 @@ protected void preStart() {
    }
 
    @Override
-   protected void postStart() {
+   public void postStart() {
       CompletionStages.join(cacheManagerNotifier.notifyCacheStarted(cacheName));
    }
 

core/src/main/java/org/infinispan/factories/GlobalComponentRegistry.java

@@ -308,6 +308,7 @@ protected void preStart() {
       basicComponentRegistry.getComponent(XSiteEventsManager.class).running();
    }
 
+   @Override
    public void postStart() {
       modulesManagerStarted();
    }
@@ -328,6 +329,10 @@ private void modulesManagerStarting() {
 
    private void modulesManagerStarted() {
       for (ModuleLifecycle l : moduleLifecycles) {
+         if (state != ComponentStatus.RUNNING) {
+            log.tracef("Registry was shut down while performing postStart, ignoring remainder of moduleLifecycle instances.");
+            break;
+         }
          if (log.isTraceEnabled()) {
             log.tracef("Invoking %s.cacheManagerStarted()", l);
          }

core/src/main/java/org/infinispan/globalstate/impl/GlobalConfigurationManagerImpl.java

@@ -25,7 +25,6 @@
 import org.infinispan.configuration.parsing.ConfigurationBuilderHolder;
 import org.infinispan.configuration.parsing.ParserRegistry;
 import org.infinispan.factories.annotations.Inject;
-import org.infinispan.factories.annotations.Start;
 import org.infinispan.factories.scopes.Scope;
 import org.infinispan.factories.scopes.Scopes;
 import org.infinispan.globalstate.GlobalConfigurationManager;
@@ -82,8 +81,8 @@ static boolean isKnownScope(String scope) {
       return CACHE_SCOPE.equals(scope) || TEMPLATE_SCOPE.equals(scope);
    }
 
-   @Start
-   void start() {
+   @Override
+   public void postStart() {
       switch (configurationManager.getGlobalConfiguration().globalState().configurationStorage()) {
          case IMMUTABLE:
             this.localConfigurationManager = new ImmutableLocalConfigurationStorage();

core/src/main/java/org/infinispan/manager/DefaultCacheManager.java

@@ -47,6 +47,7 @@
 import org.infinispan.commons.internal.BlockHoundUtil;
 import org.infinispan.commons.util.FileLookupFactory;
 import org.infinispan.commons.util.Immutables;
+import org.infinispan.commons.util.Util;
 import org.infinispan.commons.util.concurrent.CompletableFutures;
 import org.infinispan.configuration.ConfigurationManager;
 import org.infinispan.configuration.cache.Configuration;
@@ -524,6 +525,10 @@ private <K, V> Cache<K, V> internalGetCache(String cacheName) {
          throw new NullPointerException("Null arguments not allowed");
 
       assertIsNotTerminated();
+      if (!globalComponentRegistry.getStatus().allowInvocations()) {
+         throw new IllegalLifecycleStateException("Cache cannot be retrieved while global registry is not running!!");
+      }
+
       String actualName = configurationManager.selectCache(cacheName);
       if (getCacheBlockingCheck != null) {
          if (actualName.equals(getCacheBlockingCheck.get())) {
@@ -552,6 +557,10 @@ private <K, V> Cache<K, V> internalGetCache(String cacheName) {
                }
                return v;
             });
+            // We were interrupted don't attempt to restart the cache
+            if (Util.getRootCause(e) instanceof InterruptedException) {
+               return null;
+            }
          }
       }
       AdvancedCache<K, V> cache = (AdvancedCache<K, V>) createCache(actualName);
@@ -773,11 +782,51 @@ private void internalStart(boolean block) {
       try {
          globalComponentRegistry.getComponent(CacheManagerJmxRegistration.class).start();
          globalComponentRegistry.start();
+         // Some caches are started during postStart and we have to extract it out
+         globalComponentRegistry.postStart();
+         // We could get a stop concurrently, in which case we only want to update to RUNNING if not
+         ComponentStatus prev = updateStatusIfPrevious(ComponentStatus.INITIALIZING, ComponentStatus.RUNNING);
+         if (prev != null) {
+            log.debugf("Cache status changed to %s whiled starting %s", prev, identifierString());
+            return;
+         }
          log.debugf("Started cache manager %s", identifierString());
       } catch (Exception e) {
-         throw new EmbeddedCacheManagerStartupException(e);
-      } finally {
-         updateStatus(globalComponentRegistry.getStatus());
+         log.failedToInitializeGlobalRegistry(e);
+
+         boolean performShutdown = false;
+         lifecycleLock.lock();
+         try {
+            // First wait if another is stopping us.. if they are we have to wait until they are done with the stop
+            while (status == ComponentStatus.STOPPING) {
+               lifecycleCondition.await();
+            }
+            // It is possible another concurrent stop happened which killed our start, so we stop only if that
+            // wasn't taking place
+            if (status != ComponentStatus.FAILED && status != ComponentStatus.TERMINATED) {
+               performShutdown = true;
+               status = ComponentStatus.STOPPING;
+               lifecycleCondition.signalAll();
+            }
+         } catch (InterruptedException ie) {
+            throw new CacheException("Interrupted waiting for the cache manager to stop");
+         } finally {
+            lifecycleLock.unlock();
+         }
+
+         if (performShutdown) {
+            log.tracef("Stopping all caches first before global component registry");
+            stopCaches();
+            try {
+               globalComponentRegistry.componentFailed(e);
+            } catch (Exception e1) {
+               // Certain tests require this exception
+               throw new EmbeddedCacheManagerStartupException(e1);
+            } finally {
+               updateStatus(ComponentStatus.FAILED);
+            }
+            throw new EmbeddedCacheManagerStartupException(e);
+         }
       }
    }
 
@@ -800,6 +849,24 @@ private void updateStatus(ComponentStatus status) {
       }
    }
 
+   /**
+    * Updates the status to the new status only if the previous status is equal. Returns null if updated otherwise
+    * returns the previous status that didn't match.
+    */
+   private ComponentStatus updateStatusIfPrevious(ComponentStatus prev, ComponentStatus status) {
+      lifecycleLock.lock();
+      try {
+         if (this.status == prev) {
+            this.status = status;
+            lifecycleCondition.signalAll();
+            return null;
+         }
+         return this.status;
+      } finally {
+         lifecycleLock.unlock();
+      }
+   }
+
    private void terminate(String cacheName) {
       CompletableFuture<Cache<?, ?>> cacheFuture = this.caches.get(cacheName);
       if (cacheFuture != null) {

core/src/main/java/org/infinispan/security/actions/SecurityActions.java

@@ -175,6 +175,13 @@ public static void stopManager(EmbeddedCacheManager cacheManager) {
       });
    }
 
+   public static void startManager(EmbeddedCacheManager cacheManager) {
+      doPrivileged(() -> {
+         cacheManager.start();
+         return null;
+      });
+   }
+
    public static RaftManager getRaftManager(EmbeddedCacheManager ecm) {
       return doPrivileged(new GetRaftManagerAction(ecm));
    }