Move transaction processing into session close

Fixes: #15223
This commit is contained in:
Hynek Mlnarik 2022-10-31 18:12:07 +01:00 committed by Hynek Mlnařík
parent f7ad00270e
commit 071fc03f41
19 changed files with 157 additions and 204 deletions

View file

@ -31,7 +31,19 @@ public class StackUtil {
return getShortStackTrace("\n ");
}
private static final Pattern IGNORED = Pattern.compile("sun\\.|java\\.(lang|util|stream)\\.|org\\.jboss\\.(arquillian|logging).|org.apache.maven.surefire|org\\.junit\\.|org.keycloak.testsuite.model.KeycloakModelTest\\.");
private static final Pattern IGNORED = Pattern.compile("sun\\.|"
+ "java\\.(lang|util|stream)\\.|"
+ "jdk\\.internal\\.|"
+ "org\\.jboss\\.(arquillian|logging|logmanager|threads).|"
+ "org.apache.maven.surefire|"
+ "org\\.xnio\\.|"
+ "org\\.junit\\.|"
+ "org\\.infinispan\\.(interceptors|cache|notifications\\.cachelistener)\\.|"
+ "io\\.quarkus\\.|"
+ "io\\.undertow\\.|"
+ "picocli\\.|"
+ "org.keycloak.testsuite.model.KeycloakModelTest\\."
);
private static final StringBuilder EMPTY = new StringBuilder(0);
/**

View file

@ -44,8 +44,6 @@ public class ClusterAwareScheduledTaskRunner extends ScheduledTaskRunner {
@Override
protected void runTask(final KeycloakSession session) {
session.getTransactionManager().begin();
ClusterProvider clusterProvider = session.getProvider(ClusterProvider.class);
String taskKey = task.getClass().getSimpleName();
@ -61,8 +59,6 @@ public class ClusterAwareScheduledTaskRunner extends ScheduledTaskRunner {
});
session.getTransactionManager().commit();
if (result.isExecuted()) {
logger.debugf("Executed scheduled task %s", taskKey);
} else {

View file

@ -98,8 +98,7 @@ public class LegacyDatastoreProviderFactory implements DatastoreProviderFactory,
public static void setupScheduledTasks(final KeycloakSessionFactory sessionFactory) {
long interval = Config.scope("scheduled").getLong("interval", 900L) * 1000;
KeycloakSession session = sessionFactory.create();
try {
try (KeycloakSession session = sessionFactory.create()) {
TimerProvider timer = session.getProvider(TimerProvider.class);
if (timer != null) {
timer.schedule(new ClusterAwareScheduledTaskRunner(sessionFactory, new ClearExpiredEvents(), interval), interval, "ClearExpiredEvents");
@ -108,8 +107,6 @@ public class LegacyDatastoreProviderFactory implements DatastoreProviderFactory,
timer.schedule(new ScheduledTaskRunner(sessionFactory, new ClearExpiredUserSessions()), interval, ClearExpiredUserSessions.TASK_NAME);
UserStorageSyncManager.bootstrapPeriodic(sessionFactory, timer);
}
} finally {
session.close();
}
}

View file

@ -41,6 +41,7 @@ import org.keycloak.Config;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.KeycloakTransactionManager;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler;
import org.keycloak.quarkus.runtime.cli.Picocli;
import org.keycloak.common.Version;
@ -159,24 +160,16 @@ public class KeycloakMain implements QuarkusApplication {
}
KeycloakSessionFactory sessionFactory = KeycloakApplication.getSessionFactory();
KeycloakSession session = sessionFactory.create();
KeycloakTransactionManager transaction = session.getTransactionManager();
try {
transaction.begin();
KeycloakModelUtils.runJobInTransaction(sessionFactory, session -> {
new ApplianceBootstrap(session).createMasterRealmUser(adminUserName, adminPassword);
ServicesLogger.LOGGER.addUserSuccess(adminUserName, Config.getAdminRealm());
transaction.commit();
});
} catch (IllegalStateException e) {
session.getTransactionManager().rollback();
ServicesLogger.LOGGER.addUserFailedUserExists(adminUserName, Config.getAdminRealm());
} catch (Throwable t) {
session.getTransactionManager().rollback();
ServicesLogger.LOGGER.addUserFailed(t, adminUserName, Config.getAdminRealm());
} finally {
session.close();
}
}
}

View file

@ -109,13 +109,12 @@ public class LegacyJpaConnectionProviderFactory extends AbstractJpaConnectionPro
public void postInit(KeycloakSessionFactory factory) {
super.postInit(factory);
KeycloakSession session = factory.create();
String id = null;
String version = null;
String schema = getSchema();
boolean schemaChanged;
try (Connection connection = getConnection()) {
try (Connection connection = getConnection(); KeycloakSession session = factory.create()) {
try {
try (Statement statement = connection.createStatement()) {
try (ResultSet rs = statement.executeQuery(String.format(SQL_GET_LATEST_VERSION, getSchema(schema)))) {
@ -133,8 +132,6 @@ public class LegacyJpaConnectionProviderFactory extends AbstractJpaConnectionPro
schemaChanged = createOrUpdateSchema(schema, version, connection, session);
} catch (SQLException cause) {
throw new RuntimeException("Failed to update database.", cause);
} finally {
session.close();
}
if (schemaChanged || Environment.isImportExportMode()) {

View file

@ -39,8 +39,7 @@ public interface TransactionalSessionHandler {
default KeycloakSession create() {
KeycloakSessionFactory sessionFactory = getSessionFactory();
KeycloakSession session = sessionFactory.create();
KeycloakTransactionManager tx = session.getTransactionManager();
tx.begin();
session.getTransactionManager().begin();
return session;
}
@ -54,18 +53,6 @@ public interface TransactionalSessionHandler {
return;
}
KeycloakTransactionManager tx = session.getTransactionManager();
try {
if (tx.isActive()) {
if (tx.getRollbackOnly()) {
tx.rollback();
} else {
tx.commit();
}
}
} finally {
session.close();
}
}
}

View file

@ -41,7 +41,6 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.KeycloakSessionTask;
import org.keycloak.models.KeycloakSessionTaskWithResult;
import org.keycloak.models.KeycloakTransaction;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RealmProvider;
import org.keycloak.models.RoleModel;
@ -263,28 +262,16 @@ public final class KeycloakModelUtils {
* Wrap a given callable job into a KeycloakTransaction.
*/
public static <V> V runJobInTransactionWithResult(KeycloakSessionFactory factory, final KeycloakSessionTaskWithResult<V> callable) {
KeycloakSession session = factory.create();
KeycloakTransaction tx = session.getTransactionManager();
V result;
try (KeycloakSession session = factory.create()) {
session.getTransactionManager().begin();
try {
tx.begin();
result = callable.run(session);
if (tx.isActive()) {
if (tx.getRollbackOnly()) {
tx.rollback();
} else {
tx.commit();
} catch (Throwable t) {
session.getTransactionManager().setRollbackOnly();
throw t;
}
}
} catch (RuntimeException re) {
if (tx.isActive()) {
tx.rollback();
}
throw re;
} finally {
session.close();
}
return result;
}
@ -306,25 +293,11 @@ public final class KeycloakModelUtils {
final int attemptsCount, final int retryIntervalMillis) {
int retryCount = 0;
Random rand = new Random();
V result;
while (true) {
KeycloakSession session = factory.create();
KeycloakTransaction tx = session.getTransactionManager();
try {
tx.begin();
result = callable.run(session);
if (tx.isActive()) {
if (tx.getRollbackOnly()) {
tx.rollback();
} else {
tx.commit();
}
}
break;
try (KeycloakSession session = factory.create()) {
session.getTransactionManager().begin();
return callable.run(session);
} catch (RuntimeException re) {
if (tx.isActive()) {
tx.rollback();
}
if (isExceptionRetriable(re) && ++retryCount < attemptsCount) {
int delay = Math.min(retryIntervalMillis * attemptsCount, (1 << retryCount) * retryIntervalMillis)
+ rand.nextInt(retryIntervalMillis);
@ -341,11 +314,8 @@ public final class KeycloakModelUtils {
}
throw re;
}
} finally {
session.close();
}
}
return result;
}
/**

View file

@ -46,32 +46,27 @@ public class ScheduledTaskRunner implements Runnable {
@Override
public void run() {
KeycloakSession session = sessionFactory.create();
try {
KeycloakModelUtils.runJobInTransaction(sessionFactory, session -> {
try {
if (transactionLimit != 0) {
KeycloakModelUtils.setTransactionLimit(sessionFactory, transactionLimit);
}
runTask(session);
} catch (Throwable t) {
logger.errorf(t, "Failed to run scheduled task %s", task.getClass().getSimpleName());
session.getTransactionManager().rollback();
runTask(session);
} finally {
if (transactionLimit != 0) {
KeycloakModelUtils.setTransactionLimit(sessionFactory, 0);
}
try {
session.close();
} catch (Throwable t) {
logger.errorf(t, "Failed to close ProviderSession");
}
});
} catch (Throwable t) {
logger.errorf(t, "Failed to run scheduled task %s", task.getClass().getSimpleName());
}
}
protected void runTask(KeycloakSession session) {
session.getTransactionManager().begin();
task.run(session);
session.getTransactionManager().commit();
logger.debug("Executed scheduled task " + task.getClass().getSimpleName());
}

View file

@ -32,7 +32,7 @@ import java.util.function.Function;
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public interface KeycloakSession {
public interface KeycloakSession extends AutoCloseable {
KeycloakContext getContext();

View file

@ -88,13 +88,13 @@ public class DefaultKeycloakSession implements KeycloakSession {
private TokenManager tokenManager;
private VaultTranscriber vaultTranscriber;
private ClientPolicyManager clientPolicyManager;
private boolean closed;
private boolean closed = false;
public DefaultKeycloakSession(DefaultKeycloakSessionFactory factory) {
this.factory = factory;
this.transactionManager = new DefaultKeycloakTransactionManager(this);
context = createKeycloakContext(this);
LOG.tracef("Created %s%s", this, StackUtil.getShortStackTrace());
}
@Override
@ -452,11 +452,23 @@ public class DefaultKeycloakSession implements KeycloakSession {
return clientPolicyManager;
}
private static final Logger LOG = Logger.getLogger(DefaultKeycloakSession.class);
@Override
public void close() {
if (closed) {
throw new IllegalStateException("Illegal call to #close() on already closed KeycloakSession");
if (LOG.isTraceEnabled()) {
LOG.tracef("Closing %s%s%s", this,
getTransactionManager().isActive() ? " (transaction active" + (getTransactionManager().getRollbackOnly() ? ", ROLLBACK-ONLY" : "") + ")" : "",
StackUtil.getShortStackTrace());
}
if (closed) {
throw new IllegalStateException("Illegal call to #close() on already closed " + this);
}
RuntimeException re = closeTransactionManager();
try {
Consumer<? super Provider> safeClose = p -> {
try {
p.close();
@ -469,14 +481,43 @@ public class DefaultKeycloakSession implements KeycloakSession {
for (Entry<InvalidableObjectType, Set<Object>> me : invalidationMap.entrySet()) {
factory.invalidate(this, me.getKey(), me.getValue().toArray());
}
closed = true;
} finally {
this.closed = true;
}
public boolean isClosed() {
return closed;
if (re != null) {
throw re;
}
}
protected RuntimeException closeTransactionManager() {
if (! this.transactionManager.isActive()) {
return null;
}
try {
if (this.transactionManager.getRollbackOnly()) {
this.transactionManager.rollback();
} else {
this.transactionManager.commit();
}
} catch (RuntimeException re) {
return re;
}
return null;
}
@Override
public String toString() {
return String.format("session @ %08x", System.identityHashCode(this));
}
protected DefaultKeycloakContext createKeycloakContext(KeycloakSession session) {
return new DefaultKeycloakContext(session);
}
public boolean isClosed() {
return closed;
}
}

View file

@ -18,7 +18,6 @@ package org.keycloak.services;
import org.jboss.logging.Logger;
import org.keycloak.Config;
import org.keycloak.common.Profile;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.component.ComponentFactoryProvider;
import org.keycloak.component.ComponentFactoryProviderFactory;

View file

@ -56,15 +56,6 @@ public abstract class AbstractRequestFilter {
}
protected void close(KeycloakSession session) {
KeycloakTransactionManager tx = session.getTransactionManager();
if (tx.isActive()) {
if (tx.getRollbackOnly()) {
tx.rollback();
} else {
tx.commit();
}
}
session.close();
}

View file

@ -213,7 +213,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
events.add(take);
queue.drainTo(events, TRANSACTION_SIZE);
Collections.sort(events); // we sort to avoid deadlock due to ordered updates. Maybe I'm overthinking this.
KeycloakSession session = factory.create();
try (KeycloakSession session = factory.create()) {
session.getTransactionManager().begin();
try {
for (LoginEvent event : events) {
@ -225,10 +225,10 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
run = false;
}
}
session.getTransactionManager().commit();
} catch (Exception e) {
session.getTransactionManager().rollback();
session.getTransactionManager().setRollbackOnly();
throw e;
}
} finally {
for (LoginEvent event : events) {
if (event instanceof FailedLogin) {
@ -238,7 +238,6 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
}
}
events.clear();
session.close();
}
} catch (Exception e) {
ServicesLogger.LOGGER.failedProcessingType(e);

View file

@ -295,9 +295,8 @@ public class KeycloakApplication extends Application {
}
public void importRealm(RealmRepresentation rep, String from) {
KeycloakSession session = sessionFactory.create();
boolean exists = false;
try {
try (KeycloakSession session = sessionFactory.create()) {
session.getTransactionManager().begin();
try {
@ -316,16 +315,15 @@ public class KeycloakApplication extends Application {
RealmModel realm = manager.importRealm(rep);
ServicesLogger.LOGGER.importedRealm(realm.getName(), from);
}
session.getTransactionManager().commit();
} catch (Throwable t) {
session.getTransactionManager().rollback();
session.getTransactionManager().setRollbackOnly();
throw t;
}
} catch (Throwable t) {
if (!exists) {
ServicesLogger.LOGGER.unableToImportRealm(t, rep.getRealm(), from);
}
}
} finally {
session.close();
}
}
public void importAddUser() {
@ -346,10 +344,8 @@ public class KeycloakApplication extends Application {
for (RealmRepresentation realmRep : realms) {
for (UserRepresentation userRep : realmRep.getUsers()) {
KeycloakSession session = sessionFactory.create();
try {
session.getTransactionManager().begin();
KeycloakModelUtils.runJobInTransaction(sessionFactory, session -> {
RealmModel realm = session.realms().getRealmByName(realmRep.getRealm());
if (realm == null) {
@ -367,16 +363,11 @@ public class KeycloakApplication extends Application {
RepresentationToModel.createRoleMappings(userRep, user, realm);
ServicesLogger.LOGGER.addUserSuccess(userRep.getUsername(), realmRep.getRealm());
}
session.getTransactionManager().commit();
});
} catch (ModelDuplicateException e) {
session.getTransactionManager().rollback();
ServicesLogger.LOGGER.addUserFailedUserExists(userRep.getUsername(), realmRep.getRealm());
} catch (Throwable t) {
session.getTransactionManager().rollback();
ServicesLogger.LOGGER.addUserFailed(t, userRep.getUsername(), realmRep.getRealm());
} finally {
session.close();
}
}
}

View file

@ -232,15 +232,11 @@ public class KeycloakOnUndertow implements DeployableContainer<KeycloakOnUnderto
}
protected void setupDevConfig() {
KeycloakSession session = sessionFactory.create();
try {
try (KeycloakSession session = sessionFactory.create()) {
session.getTransactionManager().begin();
if (new ApplianceBootstrap(session).isNoMasterUser()) {
new ApplianceBootstrap(session).createMasterRealmUser("admin", "admin");
}
session.getTransactionManager().commit();
} finally {
session.close();
}
}

View file

@ -258,6 +258,9 @@ public class KeycloakQuarkusServerDeployableContainer implements DeployableConta
}
addStorageOptions(storeProvider, commands);
if (System.getProperty("auth.server.quarkus.log-level") != null) {
commands.add("--log-level=" + System.getProperty("auth.server.quarkus.log-level"));
}
commands.addAll(getAdditionalBuildArgs());

View file

@ -92,23 +92,17 @@ public class ImportTest extends AbstractTestRealmKeycloakTest {
// Need a new thread to not get context from thread processing request to run-on-server endpoint
Thread t = new Thread(() -> {
try {
KeycloakSession ses = session.getKeycloakSessionFactory().create();
RealmModel realmModel;
try (KeycloakSession ses = session.getKeycloakSessionFactory().create()) {
ses.getContext().setRealm(session.getContext().getRealm());
ses.getTransactionManager().begin();
RealmModel realmModel = new RealmManager(ses).importRealm(testRealm);
ses.getTransactionManager().commit();
ses.close();
ses = session.getKeycloakSessionFactory().create();
realmModel = new RealmManager(ses).importRealm(testRealm);
}
try (KeycloakSession ses = session.getKeycloakSessionFactory().create()) {
ses.getTransactionManager().begin();
session.realms().removeRealm(realmModel.getId());
ses.getTransactionManager().commit();
ses.close();
} catch (Throwable th) {
err.set(th);
}

View file

@ -529,13 +529,14 @@ public abstract class KeycloakModelTest {
}
protected <T> void inRolledBackTransaction(T parameter, BiConsumer<KeycloakSession, T> what) {
KeycloakSession session = getFactory().create();
try (KeycloakSession session = getFactory().create()) {
session.getTransactionManager().begin();
what.accept(session, parameter);
session.getTransactionManager().rollback();
}
}
protected <T, R> R inComittedTransaction(T parameter, BiFunction<KeycloakSession, T, R> what) {
return inComittedTransaction(parameter, what, null, null);

View file

@ -354,10 +354,9 @@ public class KeycloakServer {
}
public void importRealm(RealmRepresentation rep) {
KeycloakSession session = sessionFactory.create();;
session.getTransactionManager().begin();
try {
try (KeycloakSession session = sessionFactory.create()) {
session.getTransactionManager().begin();
RealmManager manager = new RealmManager(session);
if (rep.getId() != null && manager.getRealm(rep.getId()) != null) {
@ -372,25 +371,17 @@ public class KeycloakServer {
RealmModel realm = manager.importRealm(rep);
info("Imported realm " + realm.getName());
session.getTransactionManager().commit();
} finally {
session.close();
}
}
protected void setupDevConfig() {
if (System.getProperty("keycloak.createAdminUser", "true").equals("true")) {
KeycloakSession session = sessionFactory.create();
try {
try (KeycloakSession session = sessionFactory.create()) {
session.getTransactionManager().begin();
if (new ApplianceBootstrap(session).isNoMasterUser()) {
new ApplianceBootstrap(session).createMasterRealmUser("admin", "admin");
log.info("Created master user with credentials admin:admin");
}
session.getTransactionManager().commit();
} finally {
session.close();
}
}
}