From ec061e77edc63995f0fc3675b7973264137ce0b7 Mon Sep 17 00:00:00 2001 From: Michal Hajas Date: Fri, 1 Dec 2023 17:40:56 +0100 Subject: [PATCH] Remove GlobalLockProviderSpi (#25206) Closes #24103 Signed-off-by: Michal Hajas --- .../dblock/DBLockGlobalLockProvider.java | 104 --------- .../DBLockGlobalLockProviderFactory.java | 50 ---- ...k.models.locking.GlobalLockProviderFactory | 18 -- .../LegacyJpaConnectionProviderFactory.java | 26 ++- .../models/locking/GlobalLockProvider.java | 97 -------- .../locking/GlobalLockProviderFactory.java | 23 -- .../models/locking/GlobalLockProviderSpi.java | 47 ---- .../LockAcquiringTimeoutException.java | 69 ------ .../services/org.keycloak.provider.Spi | 1 - services/pom.xml | 4 + .../resources/KeycloakApplication.java | 27 +-- .../resources/META-INF/keycloak-server.json | 4 +- .../testsuite/model/KeycloakModelTest.java | 2 - .../model/globalLock/GlobalLocksTest.java | 218 ------------------ .../testsuite/model/parameters/LegacyJpa.java | 4 - .../resources/META-INF/keycloak-server.json | 4 +- 16 files changed, 35 insertions(+), 663 deletions(-) delete mode 100644 model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProvider.java delete mode 100644 model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProviderFactory.java delete mode 100644 model/legacy-private/src/main/resources/META-INF/services/org.keycloak.models.locking.GlobalLockProviderFactory delete mode 100644 server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProvider.java delete mode 100644 server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderFactory.java delete mode 100644 server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderSpi.java delete mode 100644 server-spi-private/src/main/java/org/keycloak/models/locking/LockAcquiringTimeoutException.java delete mode 100644 testsuite/model/src/test/java/org/keycloak/testsuite/model/globalLock/GlobalLocksTest.java diff --git a/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProvider.java b/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProvider.java deleted file mode 100644 index 476cc7dab6..0000000000 --- a/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProvider.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.dblock; - -import org.jboss.logging.Logger; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionTaskWithResult; -import org.keycloak.models.locking.GlobalLockProvider; - -import java.time.Duration; -import java.util.Objects; - -import static org.keycloak.models.locking.GlobalLockProvider.Constants.KEYCLOAK_BOOT; - -public class DBLockGlobalLockProvider implements GlobalLockProvider { - - private static final Logger LOG = Logger.getLogger(DBLockGlobalLockProvider.class); - public static final String DATABASE = "database"; - private final KeycloakSession session; - private final DBLockProvider dbLockProvider; - public DBLockGlobalLockProvider(KeycloakSession session, DBLockProvider dbLockProvider) { - this.session = session; - this.dbLockProvider = dbLockProvider; - } - - private static DBLockProvider.Namespace stringToNamespace(String lockName) { - switch (lockName) { - case DATABASE: - return DBLockProvider.Namespace.DATABASE; - case KEYCLOAK_BOOT: - return DBLockProvider.Namespace.KEYCLOAK_BOOT; - default: - throw new RuntimeException("Lock with name " + lockName + " not supported by DBLockGlobalLockProvider."); - } - } - - /** - * Acquires a new non-reentrant global lock that is visible to all Keycloak nodes. If the lock was successfully - * acquired the method runs the {@code task} and return result to the caller. - *

- * See {@link GlobalLockProvider#withLock(String, Duration, KeycloakSessionTaskWithResult)} for more details. - *

- * This implementation does NOT meet all requirements from the JavaDoc of {@link GlobalLockProvider#withLock(String, Duration, KeycloakSessionTaskWithResult)} - * because {@link DBLockProvider} does not provide a way to lock and unlock in separate transactions. - * Also, the database schema update currently requires to be running in the same thread that initiated the update - * therefore the {@code task} is also running in the caller thread/transaction. - */ - @Override - public V withLock(String lockName, Duration timeToWaitForLock, KeycloakSessionTaskWithResult task) { - Objects.requireNonNull(lockName, "lockName cannot be null"); - - if (timeToWaitForLock != null) { - LOG.debug("DBLockGlobalLockProvider does not support setting timeToWaitForLock per lock."); - } - - if (dbLockProvider.getCurrentLock() != null) { - throw new IllegalStateException("this lock is not reentrant, already locked for " + dbLockProvider.getCurrentLock()); - } - - dbLockProvider.waitForLock(stringToNamespace(lockName)); - try { - return task.run(session); - } finally { - releaseLock(lockName); - } - } - - private void releaseLock(String lockName) { - if (dbLockProvider.getCurrentLock() != stringToNamespace(lockName)) { - throw new RuntimeException("Requested releasing lock with name " + lockName + ", but lock is currently acquired for " + dbLockProvider.getCurrentLock() + "."); - } - - dbLockProvider.releaseLock(); - } - - @Override - public void forceReleaseAllLocks() { - if (dbLockProvider.supportsForcedUnlock()) { - dbLockProvider.releaseLock(); - } else { - throw new IllegalStateException("Forced unlock requested, but provider " + dbLockProvider + " does not support it."); - } - } - - @Override - public void close() { - - } -} diff --git a/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProviderFactory.java b/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProviderFactory.java deleted file mode 100644 index 27aa9ff56d..0000000000 --- a/model/legacy-private/src/main/java/org/keycloak/models/dblock/DBLockGlobalLockProviderFactory.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.dblock; - -import org.keycloak.Config; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.locking.GlobalLockProvider; -import org.keycloak.models.locking.GlobalLockProviderFactory; - -public class DBLockGlobalLockProviderFactory implements GlobalLockProviderFactory { - - public static final String PROVIDER_ID = "dblock"; - - @Override - public GlobalLockProvider create(KeycloakSession session) { - DBLockManager dbLockManager = new DBLockManager(session); - dbLockManager.checkForcedUnlock(); - return new DBLockGlobalLockProvider(session, dbLockManager.getDBLock()); - } - - @Override - public void init(Config.Scope config) { } - - @Override - public void postInit(KeycloakSessionFactory factory) { } - - @Override - public void close() { } - - @Override - public String getId() { - return PROVIDER_ID; - } -} diff --git a/model/legacy-private/src/main/resources/META-INF/services/org.keycloak.models.locking.GlobalLockProviderFactory b/model/legacy-private/src/main/resources/META-INF/services/org.keycloak.models.locking.GlobalLockProviderFactory deleted file mode 100644 index 1aa57e0905..0000000000 --- a/model/legacy-private/src/main/resources/META-INF/services/org.keycloak.models.locking.GlobalLockProviderFactory +++ /dev/null @@ -1,18 +0,0 @@ -# -# Copyright 2021 Red Hat, Inc. and/or its affiliates -# and other contributors as indicated by the @author tags. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -org.keycloak.models.dblock.DBLockGlobalLockProviderFactory diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/database/LegacyJpaConnectionProviderFactory.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/database/LegacyJpaConnectionProviderFactory.java index 5ff2758a39..7cabbaf462 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/database/LegacyJpaConnectionProviderFactory.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/database/LegacyJpaConnectionProviderFactory.java @@ -49,8 +49,8 @@ import org.keycloak.migration.MigrationModelManager; import org.keycloak.migration.ModelVersion; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.dblock.DBLockGlobalLockProvider; -import org.keycloak.models.locking.GlobalLockProvider; +import org.keycloak.models.dblock.DBLockManager; +import org.keycloak.models.dblock.DBLockProvider; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderConfigurationBuilder; import org.keycloak.provider.ServerInfoAwareProviderFactory; @@ -285,19 +285,25 @@ public class LegacyJpaConnectionProviderFactory extends AbstractJpaConnectionPro } private void update(Connection connection, String schema, KeycloakSession session, JpaUpdaterProvider updater) { - GlobalLockProvider globalLock = session.getProvider(GlobalLockProvider.class); - globalLock.withLock(DBLockGlobalLockProvider.DATABASE, innerSession -> { + DBLockManager dbLockManager = new DBLockManager(session); + DBLockProvider dbLock2 = dbLockManager.getDBLock(); + dbLock2.waitForLock(DBLockProvider.Namespace.DATABASE); + try { updater.update(connection, schema); - return null; - }); + } finally { + dbLock2.releaseLock(); + } } private void export(Connection connection, String schema, File databaseUpdateFile, KeycloakSession session, JpaUpdaterProvider updater) { - GlobalLockProvider globalLock = session.getProvider(GlobalLockProvider.class); - globalLock.withLock(DBLockGlobalLockProvider.DATABASE, innerSession -> { + DBLockManager dbLockManager = new DBLockManager(session); + DBLockProvider dbLock2 = dbLockManager.getDBLock(); + dbLock2.waitForLock(DBLockProvider.Namespace.DATABASE); + try { updater.export(connection, schema, databaseUpdateFile); - return null; - }); + } finally { + dbLock2.releaseLock(); + } } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProvider.java b/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProvider.java deleted file mode 100644 index 19542fb85b..0000000000 --- a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProvider.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.locking; - -import org.keycloak.models.KeycloakSessionTaskWithResult; -import org.keycloak.provider.Provider; - -import java.time.Duration; - -public interface GlobalLockProvider extends Provider { - - class Constants { - public static final String KEYCLOAK_BOOT = "keycloak-boot"; - } - - /** - * Acquires a new non-reentrant global lock that is visible to all Keycloak nodes. - * Effectively the same as {@code withLock(lockName, null, task)} - * - * @param lockName Identifier used for acquiring lock. Can be any non-null string. - * @param task The task that will be executed under the acquired lock - * @param Type of object returned by the {@code task} - * @return Value returned by the {@code task} - * @throws LockAcquiringTimeoutException When acquiring the global lock times out - * (see Javadoc of {@link #withLock(String, Duration, KeycloakSessionTaskWithResult)} for more details on how the time - * duration is determined) - * @throws NullPointerException When lockName is {@code null}. - */ - default V withLock(String lockName, KeycloakSessionTaskWithResult task) throws LockAcquiringTimeoutException { - return withLock(lockName, null, task); - } - - /** - * Acquires a new non-reentrant global lock that is visible to all Keycloak nodes. If the lock was successfully - * acquired the method runs the {@code task} in a new transaction to ensure all data modified in {@code task} - * is committed to the stores before releasing the lock and returning to the caller. - *

- * If there is another global lock with the same identifier ({@code lockName}) already acquired, this method waits - * until the lock is released, however, not more than {@code timeToWaitForLock} duration. If the lock is not - * acquired after {@code timeToWaitForLock} duration, the method throws {@link LockAcquiringTimeoutException}. - *

- * When the execution of the {@code task} finishes, the acquired lock must be released regardless of the result. - *

- * A note to implementors of the interface: - *

- * To make sure acquiring/releasing the lock is visible to all Keycloak nodes it may be needed to run the code that - * acquires/releases the lock in a separate transactions. This means together the method can use 3 separate - * transactions, for example: - * 

-     *     try {
-     *         KeycloakModelUtils.runJobInTransaction(factory,
-     *                                innerSession -> /* run code that acquires the lock *\/)
-     *
-     *         KeycloakModelUtils.runJobInTransactionWithResult(factory, task)
-     *     } finally {
-     *         KeycloakModelUtils.runJobInTransaction(factory,
-     *                                innerSession -> /* run code that releases the lock *\/)
-     *     }
-     *
- * - * @param lockName Identifier used for acquiring lock. Can be any non-null string. - * @param task The task that will be executed under the acquired lock - * @param Type of object returned by the {@code task} - * @param timeToWaitForLock Duration this method waits until it gives up acquiring the lock. If {@code null}, - * each implementation should provide some default duration, for example, using - * a configuration option. - * @return Value returned by the {@code task} - * - * @throws LockAcquiringTimeoutException When the method waits for {@code timeToWaitForLock} duration and the lock is still - * not available to acquire. - * @throws NullPointerException When {@code lockName} is {@code null}. - */ - V withLock(String lockName, Duration timeToWaitForLock, KeycloakSessionTaskWithResult task) throws LockAcquiringTimeoutException; - - /** - * Releases all locks acquired by this GlobalLockProvider. - *

- * This method unlocks all existing locks acquired by this provider regardless of the thread - * or Keycloak instance that originally acquired them. - */ - void forceReleaseAllLocks(); -} diff --git a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderFactory.java deleted file mode 100644 index 30575bd498..0000000000 --- a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderFactory.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.locking; - -import org.keycloak.provider.ProviderFactory; - -public interface GlobalLockProviderFactory extends ProviderFactory { -} diff --git a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderSpi.java b/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderSpi.java deleted file mode 100644 index b99cf05920..0000000000 --- a/server-spi-private/src/main/java/org/keycloak/models/locking/GlobalLockProviderSpi.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.locking; - -import org.keycloak.provider.Provider; -import org.keycloak.provider.ProviderFactory; -import org.keycloak.provider.Spi; - -public class GlobalLockProviderSpi implements Spi { - - public static final String GLOBAL_LOCK = "globalLock"; - - @Override - public boolean isInternal() { - return true; - } - - @Override - public String getName() { - return GLOBAL_LOCK; - } - - @Override - public Class getProviderClass() { - return GlobalLockProvider.class; - } - - @Override - public Class getProviderFactoryClass() { - return GlobalLockProviderFactory.class; - } -} diff --git a/server-spi-private/src/main/java/org/keycloak/models/locking/LockAcquiringTimeoutException.java b/server-spi-private/src/main/java/org/keycloak/models/locking/LockAcquiringTimeoutException.java deleted file mode 100644 index 31f783e210..0000000000 --- a/server-spi-private/src/main/java/org/keycloak/models/locking/LockAcquiringTimeoutException.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.models.locking; - -import java.time.Instant; - -/** - * This exception is thrown when acquiring a lock times out. - */ -public final class LockAcquiringTimeoutException extends RuntimeException { - - private final String lockName; - private final String keycloakInstanceIdentifier; - private final Instant timeWhenAcquired; - - /** - * - * @param lockName Identifier of a lock whose acquiring was unsuccessful. - * @param keycloakInstanceIdentifier Identifier of a Keycloak instance that is currently holding the lock. - * @param timeWhenAcquired Time instant when the lock held by {@code keycloakInstanceIdentifier} was acquired. - */ - public LockAcquiringTimeoutException(String lockName, String keycloakInstanceIdentifier, Instant timeWhenAcquired) { - super(String.format("Lock [%s] already acquired by keycloak instance [%s] at the time [%s]", lockName, keycloakInstanceIdentifier, timeWhenAcquired)); - this.lockName = lockName; - this.keycloakInstanceIdentifier = keycloakInstanceIdentifier; - this.timeWhenAcquired = timeWhenAcquired; - } - - /** - * - * @param lockName Identifier of a lock whose acquiring was unsuccessful. - * @param keycloakInstanceIdentifier Identifier of a Keycloak instance that is currently holding the lock. - * @param timeWhenAcquired Time instant when the lock held by {@code keycloakInstanceIdentifier} was acquired. - * @param cause The cause. - */ - public LockAcquiringTimeoutException(String lockName, String keycloakInstanceIdentifier, Instant timeWhenAcquired, Throwable cause) { - super(String.format("Lock [%s] already acquired by keycloak instance [%s] at the time [%s]", lockName, keycloakInstanceIdentifier, timeWhenAcquired), cause); - this.lockName = lockName; - this.keycloakInstanceIdentifier = keycloakInstanceIdentifier; - this.timeWhenAcquired = timeWhenAcquired; - } - - public String getLockName() { - return lockName; - } - - public String getKeycloakInstanceIdentifier() { - return keycloakInstanceIdentifier; - } - - public Instant getTimeWhenAcquired() { - return timeWhenAcquired; - } -} diff --git a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi index 42ca228470..3a1a5cd941 100755 --- a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi +++ b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi @@ -28,7 +28,6 @@ org.keycloak.models.SingleUseObjectSpi org.keycloak.models.UserSessionSpi org.keycloak.models.UserLoginFailureSpi org.keycloak.models.UserSpi -org.keycloak.models.locking.GlobalLockProviderSpi org.keycloak.migration.MigrationSpi org.keycloak.events.EventListenerSpi org.keycloak.events.EventStoreSpi diff --git a/services/pom.xml b/services/pom.xml index 7e1600eee2..9a58c98c1e 100755 --- a/services/pom.xml +++ b/services/pom.xml @@ -205,6 +205,10 @@ io.smallrye.common smallrye-common-annotation + + org.keycloak + keycloak-model-legacy-private + diff --git a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java index bac0af8dab..904c82a44c 100644 --- a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java +++ b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java @@ -33,7 +33,8 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserProvider; -import org.keycloak.models.locking.GlobalLockProvider; +import org.keycloak.models.dblock.DBLockManager; +import org.keycloak.models.dblock.DBLockProvider; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.PostMigrationEvent; import org.keycloak.models.utils.RepresentationToModel; @@ -131,24 +132,18 @@ public class KeycloakApplication extends Application { ExportImportManager[] exportImportManager = new ExportImportManager[1]; - // Release all locks acquired by currently used GlobalLockProvider if keycloak.globalLock.forceUnlock is equal - // to true. This can be used to recover from a state where there are some stale locks that were not correctly - // unlocked - if (Boolean.getBoolean("keycloak.globalLock.forceUnlock")) { - KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { - @Override - public void run(KeycloakSession session) { - GlobalLockProvider locks = session.getProvider(GlobalLockProvider.class); - locks.forceReleaseAllLocks(); - } - }); - } - KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { @Override public void run(KeycloakSession session) { - GlobalLockProvider locks = session.getProvider(GlobalLockProvider.class); - exportImportManager[0] = locks.withLock(GlobalLockProvider.Constants.KEYCLOAK_BOOT, innerSession -> bootstrap()); + DBLockManager dbLockManager = new DBLockManager(session); + dbLockManager.checkForcedUnlock(); + DBLockProvider dbLock = dbLockManager.getDBLock(); + dbLock.waitForLock(DBLockProvider.Namespace.KEYCLOAK_BOOT); + try { + exportImportManager[0] = bootstrap(); + } finally { + dbLock.releaseLock(); + } } }); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json index 0328a3d11b..e8831447fb 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json @@ -40,8 +40,8 @@ "provider": "${keycloak.deploymentState.provider:jpa}" }, - "globalLock": { - "provider": "${keycloak.globalLock.provider:dblock}" + "dblock": { + "provider": "${keycloak.dblock.provider:jpa}" }, "realm": { 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 a7a338de68..9f0d80422b 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 @@ -45,7 +45,6 @@ import org.keycloak.models.DeploymentStateSpi; import org.keycloak.models.UserLoginFailureSpi; import org.keycloak.models.UserSessionSpi; import org.keycloak.models.UserSpi; -import org.keycloak.models.locking.GlobalLockProviderSpi; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.PostMigrationEvent; import org.keycloak.provider.Provider; @@ -233,7 +232,6 @@ public abstract class KeycloakModelTest { .add(ClientSpi.class) .add(ComponentFactorySpi.class) .add(ClusterSpi.class) - .add(GlobalLockProviderSpi.class) .add(EventStoreSpi.class) .add(ExecutorsSpi.class) .add(GroupSpi.class) diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/globalLock/GlobalLocksTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/globalLock/GlobalLocksTest.java deleted file mode 100644 index 7f07f84f4c..0000000000 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/globalLock/GlobalLocksTest.java +++ /dev/null @@ -1,218 +0,0 @@ -/* - * Copyright 2022 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.testsuite.model.globalLock; - -import org.hamcrest.Matchers; -import org.jboss.logging.Logger; -import org.junit.Test; -import org.keycloak.models.dblock.DBLockGlobalLockProviderFactory; -import org.keycloak.models.locking.GlobalLockProvider; -import org.keycloak.models.locking.LockAcquiringTimeoutException; -import org.keycloak.testsuite.model.KeycloakModelTest; -import org.keycloak.testsuite.model.RequireProvider; - -import java.time.Duration; -import java.util.LinkedList; -import java.util.List; -import java.util.Random; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; -import java.util.stream.IntStream; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.hasSize; - -@RequireProvider(value = GlobalLockProvider.class, - exclude = { DBLockGlobalLockProviderFactory.PROVIDER_ID } -) -public class GlobalLocksTest extends KeycloakModelTest { - - private static final Logger LOG = Logger.getLogger(GlobalLocksTest.class); - @Override - protected boolean isUseSameKeycloakSessionFactoryForAllThreads() { - return true; - } - - @Test - public void concurrentLockingTest() { - final String LOCK_NAME = "simpleLockTestLockName"; - - AtomicInteger counter = new AtomicInteger(); - int numIterations = 50; - Random rand = new Random(); - List resultingList = new LinkedList<>(); - - IntStream.range(0, numIterations).parallel().forEach(index -> inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - LOG.infof("Iteration %d entered session", index); - - lockProvider.withLock(LOCK_NAME, Duration.ofSeconds(60), innerSession -> { - LOG.infof("Iteration %d entered locked block", index); - - // Locked block - int c = counter.getAndIncrement(); - - try { - Thread.sleep(rand.nextInt(100)); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException(e); - } - - resultingList.add(c); - return null; - }); - })); - - assertThat(resultingList, hasSize(numIterations)); - assertThat(resultingList, equalTo(IntStream.range(0, 50).boxed().collect(Collectors.toList()))); - } - - @Test - public void lockTimeoutExceptionTest() { - final String LOCK_NAME = "lockTimeoutExceptionTestLock"; - AtomicInteger counter = new AtomicInteger(); - CountDownLatch waitForTheOtherThreadToFail = new CountDownLatch(1); - - IntStream.range(0, 2).parallel().forEach(index -> inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - - try { - lockProvider.withLock(LOCK_NAME, Duration.ofSeconds(2), innerSession -> { - int c = counter.incrementAndGet(); - if (c == 1) { - try { - waitForTheOtherThreadToFail.await(); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - } else { - LOG.infof("Lock acquired by thread %s with counter: %d", Thread.currentThread().getName(), c); - throw new RuntimeException("Lock acquired by more than one thread."); - } - return null; - }); - } catch (LockAcquiringTimeoutException e) { - int c = counter.incrementAndGet(); - LOG.infof("Exception when acquiring lock by thread %s with counter: %d", Thread.currentThread().getName(), c); - if (c != 2) { - throw new RuntimeException("Acquiring lock failed by different thread than second."); - } - - assertThat(e.getMessage(), containsString("Lock [" + LOCK_NAME + "] already acquired by keycloak instance")); - waitForTheOtherThreadToFail.countDown(); - } - })); - } - - @Test - public void testReleaseAllLocksMethod() throws InterruptedException { - final int numberOfThreads = 4; - ExecutorService executor = Executors.newFixedThreadPool(numberOfThreads); - - CountDownLatch locksAcquired = new CountDownLatch(numberOfThreads); - CountDownLatch testFinished = new CountDownLatch(1); - - LOG.info("Initial locks acquiring phase."); - try { - // Acquire locks and let the threads wait until the end of this test method - for (int index = 0; index < numberOfThreads; index++) { - final int i = index; - executor.submit(() -> - inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - LOG.infof("Acquiring LOCK_%d", i); - lockProvider.withLock("LOCK_" + i, session -> { - LOG.infof("Lock LOCK_%d acquired.", i); - locksAcquired.countDown(); - try { - testFinished.await(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException(e); - } - return null; - }); - LOG.infof("Initial acquiring tx finished for lock LOCK_%d", i); - }) - ); - } - - if (!locksAcquired.await(5, TimeUnit.MINUTES)) { - throw new RuntimeException("Acquiring locks phase took too long."); - } - - LOG.info("Expecting timeouts for each lock."); - // Test no lock can be acquired because all are still hold by the executor above - AtomicInteger counter = new AtomicInteger(); - for (int index = 0; index < numberOfThreads; index++) { - final int i = index; - inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - try { - LOG.infof("Attempt to acquire LOCK_%d.", i); - lockProvider.withLock("LOCK_" + i, Duration.ofSeconds(1), is -> { - throw new RuntimeException("Acquiring lock should not succeed as it was acquired in the first transaction"); - }); - } catch (LockAcquiringTimeoutException e) { - LOG.infof("Timeout was successfully received for LOCK_%d", i); - counter.incrementAndGet(); - } - }); - } - - assertThat(counter.get(), Matchers.equalTo(numberOfThreads)); - - // Unlock all locks forcefully - inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - LOG.infof("Releasing all locks", Thread.currentThread().getName()); - lockProvider.forceReleaseAllLocks(); - }); - - // Test all locks can be acquired again - counter.set(0); - for (int index = 0; index < numberOfThreads; index++) { - final int i = index; - inComittedTransaction(s -> { - GlobalLockProvider lockProvider = s.getProvider(GlobalLockProvider.class); - try { - lockProvider.withLock("LOCK_" + i, Duration.ofSeconds(1), is -> { - LOG.infof("Lock LOCK_%d acquired again.", i); - counter.incrementAndGet(); - return null; - }); - } catch (LockAcquiringTimeoutException e) { - throw new RuntimeException(e); - } - }); - } - - assertThat(counter.get(), Matchers.equalTo(numberOfThreads)); - } finally { - testFinished.countDown(); - executor.shutdown(); - } - } -} diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LegacyJpa.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LegacyJpa.java index 74ae77c322..16434553ae 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LegacyJpa.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LegacyJpa.java @@ -25,10 +25,8 @@ import org.keycloak.connections.jpa.updater.liquibase.conn.LiquibaseConnectionPr import org.keycloak.connections.jpa.updater.liquibase.conn.LiquibaseConnectionSpi; import org.keycloak.connections.jpa.updater.liquibase.lock.LiquibaseDBLockProviderFactory; import org.keycloak.events.jpa.JpaEventStoreProviderFactory; -import org.keycloak.models.dblock.DBLockGlobalLockProviderFactory; import org.keycloak.models.dblock.DBLockSpi; import org.keycloak.models.jpa.session.JpaUserSessionPersisterProviderFactory; -import org.keycloak.models.locking.GlobalLockProviderSpi; import org.keycloak.models.session.UserSessionPersisterSpi; import org.keycloak.migration.MigrationProviderFactory; import org.keycloak.migration.MigrationSpi; @@ -88,7 +86,6 @@ public class LegacyJpa extends KeycloakModelParameters { .add(JpaUserProviderFactory.class) .add(LiquibaseConnectionProviderFactory.class) .add(LiquibaseDBLockProviderFactory.class) - .add(DBLockGlobalLockProviderFactory.class) .add(JpaUserSessionPersisterProviderFactory.class) //required for migrateModel @@ -116,7 +113,6 @@ public class LegacyJpa extends KeycloakModelParameters { .spi("realm").defaultProvider("jpa") .spi("deploymentState").defaultProvider("jpa") .spi("dblock").defaultProvider("jpa") - .spi(GlobalLockProviderSpi.GLOBAL_LOCK).defaultProvider(DBLockGlobalLockProviderFactory.PROVIDER_ID) ; } } diff --git a/testsuite/utils/src/main/resources/META-INF/keycloak-server.json b/testsuite/utils/src/main/resources/META-INF/keycloak-server.json index dd6461bf71..ba420b26dc 100755 --- a/testsuite/utils/src/main/resources/META-INF/keycloak-server.json +++ b/testsuite/utils/src/main/resources/META-INF/keycloak-server.json @@ -21,8 +21,8 @@ "provider": "${keycloak.deploymentState.provider:jpa}" }, - "globalLock": { - "provider": "${keycloak.globalLock.provider:dblock}" + "dblock": { + "provider": "${keycloak.dblock.provider:jpa}" }, "realm": {