From bd0f87fc4d281b869ef067afc6bfd8f5eaf3848b Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 24 Jul 2023 09:50:32 +0200 Subject: [PATCH] Remove Infinispan workarounds introduced to prevent deadlocks (#21862) This should no longer be necessary after the upgrade to Infinispan 14.0.13.Final and ISPN-13666 being resolved. Closes #9871 --- .../InfinispanClusterProviderFactory.java | 13 +--- .../InfinispanNotificationsManager.java | 59 ++++++------------- ...ltInfinispanConnectionProviderFactory.java | 22 ++----- 3 files changed, 24 insertions(+), 70 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 73ab3fe4c3..7cb2d29fd8 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 @@ -33,7 +33,6 @@ import org.keycloak.cluster.ClusterProviderFactory; import org.keycloak.common.Profile; import org.keycloak.common.util.Retry; import org.keycloak.common.util.Time; -import org.keycloak.connections.infinispan.DefaultInfinispanConnectionProviderFactory; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.connections.infinispan.TopologyInfo; import org.keycloak.models.KeycloakSession; @@ -221,17 +220,7 @@ 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. - 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)); - } + workCache.entrySet().removeIf(new LockEntryPredicate(removedNodesAddresses)); } } catch (Throwable t) { logger.error("caught exception in ViewChangeListener", t); 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 22d7435beb..ddf75a020b 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 @@ -51,7 +51,6 @@ import org.keycloak.cluster.ClusterListener; import org.keycloak.cluster.ClusterProvider; import org.keycloak.common.util.ConcurrentMultivaluedHashMap; import org.keycloak.common.util.Retry; -import org.keycloak.connections.infinispan.DefaultInfinispanConnectionProviderFactory; import org.keycloak.executors.ExecutorsProvider; import org.keycloak.models.KeycloakSession; import org.infinispan.client.hotrod.exceptions.HotRodClientException; @@ -71,7 +70,7 @@ public class InfinispanNotificationsManager { private final Cache workCache; - private final RemoteCache workRemoteCache; + private final RemoteCache workRemoteCache; private final String myAddress; @@ -80,7 +79,7 @@ public class InfinispanNotificationsManager { private final ExecutorService listenersExecutor; - protected InfinispanNotificationsManager(Cache workCache, RemoteCache workRemoteCache, String myAddress, String mySite, ExecutorService listenersExecutor) { + protected InfinispanNotificationsManager(Cache workCache, RemoteCache workRemoteCache, String myAddress, String mySite, ExecutorService listenersExecutor) { this.workCache = workCache; this.workRemoteCache = workRemoteCache; this.myAddress = myAddress; @@ -91,7 +90,7 @@ public class InfinispanNotificationsManager { // Create and init manager including all listeners etc public static InfinispanNotificationsManager create(KeycloakSession session, Cache workCache, String myAddress, String mySite, Set remoteStores) { - RemoteCache workRemoteCache = null; + RemoteCache workRemoteCache = null; if (!remoteStores.isEmpty()) { RemoteStore remoteStore = remoteStores.iterator().next(); @@ -158,21 +157,11 @@ public class InfinispanNotificationsManager { // Add directly to remoteCache. Will notify remote listeners on all nodes in all DCs Retry.executeWithBackoff((int iteration) -> { try { - /* - 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. - 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); - } + workRemoteCache.put(eventKey, wrappedEvent, 120, TimeUnit.SECONDS); } catch (HotRodClientException re) { - if (logger.isDebugEnabled()) { - logger.debugf(re, "Failed sending notification to remote cache '%s'. Key: '%s', iteration '%s'. Will try to retry the task", - workRemoteCache.getName(), eventKey, iteration); + if (logger.isDebugEnabled()) { + logger.debugf(re, "Failed sending notification to remote cache '%s'. Key: '%s', iteration '%s'. Will try to retry the task", + workRemoteCache.getName(), eventKey, iteration); } // Rethrow the exception. Retry will take care of handle the exception and eventually retry the operation. @@ -195,7 +184,7 @@ public class InfinispanNotificationsManager { @CacheEntryModified public void cacheEntryModified(CacheEntryModifiedEvent event) { - eventReceived(event.getKey(), event.getValue()); + eventReceived(event.getKey(), event.getNewValue()); } @CacheEntryRemoved @@ -209,30 +198,30 @@ public class InfinispanNotificationsManager { @ClientListener public class HotRodListener { - private final RemoteCache remoteCache; + private final RemoteCache remoteCache; - public HotRodListener(RemoteCache remoteCache) { + public HotRodListener(RemoteCache remoteCache) { this.remoteCache = remoteCache; } @ClientCacheEntryCreated - public void created(ClientCacheEntryCreatedEvent event) { - String key = event.getKey().toString(); + public void created(ClientCacheEntryCreatedEvent event) { + String key = event.getKey(); hotrodEventReceived(key); } @ClientCacheEntryModified - public void updated(ClientCacheEntryModifiedEvent event) { - String key = event.getKey().toString(); + public void updated(ClientCacheEntryModifiedEvent event) { + String key = event.getKey(); hotrodEventReceived(key); } @ClientCacheEntryRemoved - public void removed(ClientCacheEntryRemovedEvent event) { - String key = event.getKey().toString(); + public void removed(ClientCacheEntryRemovedEvent event) { + String key = event.getKey(); taskFinished(key, true); } @@ -241,21 +230,7 @@ public class InfinispanNotificationsManager { // TODO: Look at CacheEventConverter stuff to possibly include value in the event and avoid additional remoteCache request try { listenersExecutor.submit(() -> { - - /* - 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. - 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) { - value = remoteCache.get(key); - } - eventReceived(key, (Serializable) value); - + eventReceived(key, remoteCache.get(key)); }); } catch (RejectedExecutionException ree) { // server is shutting down or pool was terminated - don't throw errors 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 b9f543aff0..d13262901c 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 @@ -87,23 +87,13 @@ public class DefaultInfinispanConnectionProviderFactory implements InfinispanCon @Override public void close() { - /* - 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) { - if (cacheManager != null && !containerManaged) { - cacheManager.stop(); - } - if (remoteCacheProvider != null) { - remoteCacheProvider.stop(); - } - cacheManager = null; + if (cacheManager != null && !containerManaged) { + cacheManager.stop(); } + if (remoteCacheProvider != null) { + remoteCacheProvider.stop(); + } + cacheManager = null; } @Override