Use separate transactions for each bulk update of offline sessions in…

… PersisterLastSessionRefreshStore to avoid deadlocks closes keycloak#13684
olivierlemasle · Aug 23, 2022 · 254483b · 254483b
1 parent 2002fd9
commit 254483b
Showing 1 changed file with 12 additions and 10 deletions.
diff --git a/...eycloak/models/sessions/infinispan/changes/sessions/PersisterLastSessionRefreshStore.java b/...eycloak/models/sessions/infinispan/changes/sessions/PersisterLastSessionRefreshStore.java
@@ -19,13 +19,15 @@
 
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import org.jboss.logging.Logger;
 import org.keycloak.common.util.Time;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.session.UserSessionPersisterProvider;
+import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.models.utils.SessionTimeoutHelper;
 
 /**
@@ -59,19 +61,19 @@ protected void sendMessage(KeycloakSession kcSession, Map<String, SessionData> r
             logger.debugf("Updating %d userSessions with lastSessionRefresh: %d", refreshesToSend.size(), lastSessionRefresh);
         }
 
-        UserSessionPersisterProvider persister = kcSession.getProvider(UserSessionPersisterProvider.class);
-
+        // Separate transaction for each bulk update request to avoid deadlocks
         for (Map.Entry<String, Set<String>> entry : sessionIdsByRealm.entrySet()) {
-            RealmModel realm = kcSession.realms().getRealm(entry.getKey());
-
-            // Case when realm was deleted in the meantime. UserSessions were already deleted as well (callback for realm deletion)
-            if (realm == null) {
-                continue;
-            }
+            KeycloakModelUtils.runJobInTransaction(kcSession.getKeycloakSessionFactory(), (kcSession2) -> {
+                UserSessionPersisterProvider persister = kcSession2.getProvider(UserSessionPersisterProvider.class);
+                RealmModel realm = kcSession2.realms().getRealm(entry.getKey());
 
-            Set<String> userSessionIds = entry.getValue();
+                // If realm is null, it means that realm was deleted in the meantime. UserSessions were already deleted as well (callback for realm deletion)
+                if (realm != null) {
+                    Set<String> userSessionIds = new TreeSet<>(entry.getValue());
 
-            persister.updateLastSessionRefreshes(realm, lastSessionRefresh, userSessionIds, offline);
+                    persister.updateLastSessionRefreshes(realm, lastSessionRefresh, userSessionIds, offline);
+                }
+            });
         }
     }
 }