From bb6b5abfa11865785dee283a24891979ba48823a Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Fri, 26 Aug 2022 12:05:38 +0200 Subject: [PATCH] Remove Infinispan workarounds after upgrading to 13.x Closes #13962 --- .../InfinispanClusterProviderFactory.java | 14 ++- .../InfinispanNotificationsManager.java | 8 +- ...ltInfinispanConnectionProviderFactory.java | 4 +- .../testsuite/model/KeycloakModelTest.java | 113 +++--------------- 4 files changed, 35 insertions(+), 104 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanClusterProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanClusterProviderFactory.java index fad33fb93c..73ab3fe4c3 100644 --- a/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanClusterProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanClusterProviderFactory.java @@ -221,12 +221,14 @@ public class InfinispanClusterProviderFactory implements ClusterProviderFactory, } logger.debugf("Nodes %s removed from cluster. Removing tasks locked by this nodes", removedNodesAddresses.toString()); - /* - workaround for Infinispan 12.1.7.Final to prevent a deadlock while - DefaultInfinispanConnectionProviderFactory is shutting down PersistenceManagerImpl - that acquires a writeLock and this removal that acquires a readLock. - https://issues.redhat.com/browse/ISPN-13664 - */ + /* + workaround for Infinispan 12.1.7.Final to prevent a deadlock while + DefaultInfinispanConnectionProviderFactory is shutting down PersistenceManagerImpl + that acquires a writeLock and this removal that acquires a readLock. + First seen with https://issues.redhat.com/browse/ISPN-13664 and still occurs probably due to + https://issues.redhat.com/browse/ISPN-13666 in 13.0.10 + Tracked in https://github.com/keycloak/keycloak/issues/9871 + */ synchronized (DefaultInfinispanConnectionProviderFactory.class) { workCache.entrySet().removeIf(new LockEntryPredicate(removedNodesAddresses)); } diff --git a/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanNotificationsManager.java b/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanNotificationsManager.java index 0051748aa0..f260329a60 100644 --- a/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanNotificationsManager.java +++ b/model/infinispan/src/main/java/org/keycloak/cluster/infinispan/InfinispanNotificationsManager.java @@ -162,7 +162,9 @@ public class InfinispanNotificationsManager { workaround for Infinispan 12.1.7.Final to prevent a deadlock while DefaultInfinispanConnectionProviderFactory is shutting down PersistenceManagerImpl that acquires a writeLock and this put that acquires a readLock. - https://issues.redhat.com/browse/ISPN-13664 + First seen with https://issues.redhat.com/browse/ISPN-13664 and still occurs probably due to + https://issues.redhat.com/browse/ISPN-13666 in 13.0.10 + Tracked in https://github.com/keycloak/keycloak/issues/9871 */ synchronized (DefaultInfinispanConnectionProviderFactory.class) { workRemoteCache.put(eventKey, wrappedEvent, 120, TimeUnit.SECONDS); @@ -244,7 +246,9 @@ public class InfinispanNotificationsManager { workaround for Infinispan 12.1.7.Final to prevent a deadlock while DefaultInfinispanConnectionProviderFactory is shutting down PersistenceManagerImpl that acquires a writeLock and this get that acquires a readLock. - https://issues.redhat.com/browse/ISPN-13664 + First seen with https://issues.redhat.com/browse/ISPN-13664 and still occurs probably due to + https://issues.redhat.com/browse/ISPN-13666 in 13.0.10 + Tracked in https://github.com/keycloak/keycloak/issues/9871 */ Object value; synchronized (DefaultInfinispanConnectionProviderFactory.class) { diff --git a/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java index d353aacea2..f60a9eec5a 100755 --- a/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java @@ -91,7 +91,9 @@ public class DefaultInfinispanConnectionProviderFactory implements InfinispanCon workaround for Infinispan 12.1.7.Final to prevent a deadlock while DefaultInfinispanConnectionProviderFactory is shutting down PersistenceManagerImpl that acquires a writeLock and this removal that acquires a readLock. - https://issues.redhat.com/browse/ISPN-13664 + First seen with https://issues.redhat.com/browse/ISPN-13664 and still occurs probably due to + https://issues.redhat.com/browse/ISPN-13666 in 13.0.10 + Tracked in https://github.com/keycloak/keycloak/issues/9871 */ synchronized (DefaultInfinispanConnectionProviderFactory.class) { if (cacheManager != null && !containerManaged) { diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/KeycloakModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/KeycloakModelTest.java index 761fd6e771..c2e3224523 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/KeycloakModelTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/KeycloakModelTest.java @@ -16,15 +16,12 @@ */ package org.keycloak.testsuite.model; -import org.infinispan.commons.CacheConfigurationException; -import org.infinispan.manager.EmbeddedCacheManagerStartupException; import org.junit.Assert; import org.keycloak.Config.Scope; import org.keycloak.authorization.AuthorizationSpi; import org.keycloak.authorization.DefaultAuthorizationProviderFactory; import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.policy.provider.PolicySpi; -import org.keycloak.authorization.policy.provider.permission.UMAPolicyProviderFactory; import org.keycloak.authorization.store.StoreFactorySpi; import org.keycloak.cluster.ClusterSpi; import org.keycloak.common.util.Time; @@ -76,11 +73,9 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.concurrent.Semaphore; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; @@ -282,7 +277,6 @@ public abstract class KeycloakModelTest { * local to the thread that calls this method, allowing for per-thread customization. This in turn allows * testing of several parallel session factories which can be used to simulate several servers * running in parallel. - * @return */ public static KeycloakSessionFactory createKeycloakSessionFactory() { int factoryIndex = FACTORY_COUNT.incrementAndGet(); @@ -345,7 +339,7 @@ public abstract class KeycloakModelTest { /** * Runs the given {@code task} in {@code numThreads} parallel threads, each thread operating * in the context of a fresh {@link KeycloakSessionFactory} independent of each other thread. - * + *

* Will throw an exception when the thread throws an exception or if the thread doesn't complete in time. * * @see #inIndependentFactory @@ -367,53 +361,27 @@ public abstract class KeycloakModelTest { } }); try { - /* - workaround for Infinispan 12.1.7.Final to prevent an internal Infinispan NullPointerException - when multiple nodes tried to join at the same time by starting them sequentially, - although that does not catch 100% of all occurrences. - Already fixed in Infinispan 13. - https://issues.redhat.com/browse/ISPN-13231 - */ - Semaphore sem = new Semaphore(1); CountDownLatch start = new CountDownLatch(numThreads); CountDownLatch stop = new CountDownLatch(numThreads); - Callable independentTask = () -> { - AtomicBoolean locked = new AtomicBoolean(false); + Callable independentTask = () -> inIndependentFactory(() -> { + + // use the latch to ensure that all caches are online while the transaction below runs to avoid a RemoteException + start.countDown(); + start.await(); + try { - sem.acquire(); - locked.set(true); - Object val = inIndependentFactory(() -> { - sem.release(); - locked.set(false); + task.run(); - // use the latch to ensure that all caches are online while the transaction below runs to avoid a RemoteException - start.countDown(); - start.await(); - - try { - task.run(); - - // use the latch to ensure that all caches are online while the transaction above runs to avoid a RemoteException - // otherwise might fail with "Cannot wire or start components while the registry is not running" during shutdown - // https://issues.redhat.com/browse/ISPN-9761 - } finally { - stop.countDown(); - } - stop.await(); - - sem.acquire(); - locked.set(true); - return null; - }); - sem.release(); - locked.set(false); - return val; + // use the latch to ensure that all caches are online while the transaction above runs to avoid a RemoteException + // otherwise might fail with "Cannot wire or start components while the registry is not running" during shutdown + // https://issues.redhat.com/browse/ISPN-9761 } finally { - if (locked.get()) { - sem.release(); - } + stop.countDown(); } - }; + stop.await(); + + return null; + }); // submit tasks, and wait for the results without cancelling execution so that we'll be able to analyze the thread dump List> tasks = IntStream.range(0, numThreads) @@ -484,53 +452,16 @@ public abstract class KeycloakModelTest { /** * Runs the given {@code task} in a context of a fresh {@link KeycloakSessionFactory} which is created before * running the task and destroyed afterwards. - * @return */ public static T inIndependentFactory(Callable task) { if (USE_DEFAULT_FACTORY) { throw new IllegalStateException("USE_DEFAULT_FACTORY must be false to use an independent factory"); } KeycloakSessionFactory original = getFactory(); - int retries = 10; - KeycloakSessionFactory factory = null; - do { - try { - factory = createKeycloakSessionFactory(); - } catch (CacheConfigurationException | EmbeddedCacheManagerStartupException ex) { - if (retries > 0) { - /* - workaround for Infinispan 12.1.7.Final for a NullPointerException - when multiple nodes tried to join at the same time. Retry until this succeeds. - Already fixed in Infinispan 13. - https://issues.redhat.com/browse/ISPN-13231 - */ - LOG.warn("initialization failed, retrying", ex); - --retries; - } else { - throw ex; - } - } - } while (factory == null); + KeycloakSessionFactory factory = createKeycloakSessionFactory(); try { setFactory(factory); - do { - try { - return task.call(); - } catch (CacheConfigurationException | EmbeddedCacheManagerStartupException ex) { - if (retries > 0) { - /* - workaround for Infinispan 12.1.7.Final for a NullPointerException - when multiple nodes tried to join at the same time. Retry until this succeeds. - Already fixed in Infinispan 13. - https://issues.redhat.com/browse/ISPN-13231 - */ - LOG.warn("initialization failed, retrying", ex); - -- retries; - } else { - throw ex; - } - } - } while (true); + return task.call(); } catch (Exception ex) { throw new RuntimeException(ex); } finally { @@ -586,10 +517,6 @@ public abstract class KeycloakModelTest { return MODEL_PARAMETERS.stream().flatMap(mp -> mp.getParameters(clazz)).filter(Objects::nonNull); } - protected void withEach(Class parameterClazz, Consumer what) { - getParameters(parameterClazz).forEach(what); - } - protected void inRolledBackTransaction(T parameter, BiConsumer what) { KeycloakSession session = getFactory().create(); session.getTransactionManager().begin(); @@ -634,10 +561,6 @@ public abstract class KeycloakModelTest { * Convenience method for {@link #inComittedTransaction(java.util.function.Consumer)} that * obtains realm model from the session and puts it into session context before * running the {@code what} task. - * @param - * @param realmId - * @param what - * @return */ protected R withRealm(String realmId, BiFunction what) { return inComittedTransaction(session -> {