From 1e9eb69cdf71519fe584fc5c3cf46f08a2b16b4f Mon Sep 17 00:00:00 2001 From: Mingyuan Wu Date: Thu, 23 Nov 2023 12:28:08 +0800 Subject: [PATCH 1/8] Use Lock instead synchronized --- .../ibatis/cache/decorators/SoftCache.java | 13 +- .../cache/decorators/SynchronizedCache.java | 50 ++++-- .../ibatis/cache/decorators/WeakCache.java | 13 +- .../ibatis/datasource/pooled/PoolState.java | 159 ++++++++++++------ .../unpooled/UnpooledDataSource.java | 27 ++- .../AbstractEnhancedDeserializationProxy.java | 10 +- .../loader/cglib/CglibProxyFactory.java | 55 +++--- .../javassist/JavassistProxyFactory.java | 56 +++--- .../java/org/apache/ibatis/io/JBoss6VFS.java | 45 ++--- .../org/apache/ibatis/logging/LogFactory.java | 23 ++- .../apache/ibatis/session/Configuration.java | 26 ++- .../java/org/apache/ibatis/util/LockKit.java | 113 +++++++++++++ .../primitive_result_type/IbatisConfig.java | 23 +-- 13 files changed, 444 insertions(+), 169 deletions(-) create mode 100644 src/main/java/org/apache/ibatis/util/LockKit.java diff --git a/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java b/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java index daef435e890..79f0877b9d7 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java @@ -21,6 +21,7 @@ import java.util.LinkedList; import org.apache.ibatis.cache.Cache; +import org.apache.ibatis.util.LockKit; /** * Soft Reference cache decorator. @@ -74,11 +75,15 @@ public Object getObject(Object key) { delegate.removeObject(key); } else { // See #586 (and #335) modifications need more than a read lock - synchronized (hardLinksToAvoidGarbageCollection) { + LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); + lock.lock(); + try { hardLinksToAvoidGarbageCollection.addFirst(result); if (hardLinksToAvoidGarbageCollection.size() > numberOfHardLinks) { hardLinksToAvoidGarbageCollection.removeLast(); } + } finally { + lock.unlock(); } } } @@ -95,8 +100,12 @@ public Object removeObject(Object key) { @Override public void clear() { - synchronized (hardLinksToAvoidGarbageCollection) { + LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); + lock.lock(); + try { hardLinksToAvoidGarbageCollection.clear(); + } finally { + lock.unlock(); } removeGarbageCollectedItems(); delegate.clear(); diff --git a/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java b/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java index 72f1dbccf85..5e8c7a7c9d9 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2022 the original author or authors. + * Copyright 2009-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.apache.ibatis.cache.decorators; +import java.util.concurrent.locks.ReentrantReadWriteLock; + import org.apache.ibatis.cache.Cache; /** @@ -22,6 +24,7 @@ */ public class SynchronizedCache implements Cache { + private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private final Cache delegate; public SynchronizedCache(Cache delegate) { @@ -34,28 +37,53 @@ public String getId() { } @Override - public synchronized int getSize() { - return delegate.getSize(); + public int getSize() { + readWriteLock.readLock().lock(); + try { + return delegate.getSize(); + } finally { + readWriteLock.readLock().unlock(); + } } @Override - public synchronized void putObject(Object key, Object object) { - delegate.putObject(key, object); + public void putObject(Object key, Object object) { + readWriteLock.writeLock().lock(); + try { + delegate.putObject(key, object); + } finally { + readWriteLock.writeLock().unlock(); + } } @Override - public synchronized Object getObject(Object key) { - return delegate.getObject(key); + public Object getObject(Object key) { + readWriteLock.readLock().lock(); + try { + return delegate.getObject(key); + } finally { + readWriteLock.readLock().unlock(); + } } @Override - public synchronized Object removeObject(Object key) { - return delegate.removeObject(key); + public Object removeObject(Object key) { + readWriteLock.writeLock().lock(); + try { + return delegate.removeObject(key); + } finally { + readWriteLock.writeLock().unlock(); + } } @Override - public synchronized void clear() { - delegate.clear(); + public void clear() { + readWriteLock.writeLock().lock(); + try { + delegate.clear(); + } finally { + readWriteLock.writeLock().unlock(); + } } @Override diff --git a/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java b/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java index 8af6f1d0b5a..2c0964f5b27 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java @@ -21,6 +21,7 @@ import java.util.LinkedList; import org.apache.ibatis.cache.Cache; +import org.apache.ibatis.util.LockKit; /** * Weak Reference cache decorator. @@ -73,11 +74,15 @@ public Object getObject(Object key) { if (result == null) { delegate.removeObject(key); } else { - synchronized (hardLinksToAvoidGarbageCollection) { + LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); + lock.lock(); + try { hardLinksToAvoidGarbageCollection.addFirst(result); if (hardLinksToAvoidGarbageCollection.size() > numberOfHardLinks) { hardLinksToAvoidGarbageCollection.removeLast(); } + } finally { + lock.unlock(); } } } @@ -94,8 +99,12 @@ public Object removeObject(Object key) { @Override public void clear() { - synchronized (hardLinksToAvoidGarbageCollection) { + LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); + lock.lock(); + try { hardLinksToAvoidGarbageCollection.clear(); + } finally { + lock.unlock(); } removeGarbageCollectedItems(); delegate.clear(); diff --git a/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java b/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java index f17dcfe8387..4b7e6096ba0 100644 --- a/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java +++ b/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java @@ -18,11 +18,15 @@ import java.util.ArrayList; import java.util.List; +import org.apache.ibatis.util.LockKit; + /** * @author Clinton Begin */ public class PoolState { + private final LockKit.ReentrantLock reentrantLock; + protected PooledDataSource dataSource; protected final List idleConnections = new ArrayList<>(); @@ -38,79 +42,134 @@ public class PoolState { public PoolState(PooledDataSource dataSource) { this.dataSource = dataSource; + this.reentrantLock = LockKit.obtainLock(dataSource); } - public synchronized long getRequestCount() { - return requestCount; + public long getRequestCount() { + reentrantLock.lock(); + try { + return requestCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getAverageRequestTime() { - return requestCount == 0 ? 0 : accumulatedRequestTime / requestCount; + public long getAverageRequestTime() { + reentrantLock.lock(); + try { + return requestCount == 0 ? 0 : accumulatedRequestTime / requestCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getAverageWaitTime() { - return hadToWaitCount == 0 ? 0 : accumulatedWaitTime / hadToWaitCount; - + public long getAverageWaitTime() { + reentrantLock.lock(); + try { + return hadToWaitCount == 0 ? 0 : accumulatedWaitTime / hadToWaitCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getHadToWaitCount() { - return hadToWaitCount; + public long getHadToWaitCount() { + reentrantLock.lock(); + try { + return hadToWaitCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getBadConnectionCount() { - return badConnectionCount; + public long getBadConnectionCount() { + reentrantLock.lock(); + try { + return badConnectionCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getClaimedOverdueConnectionCount() { - return claimedOverdueConnectionCount; + public long getClaimedOverdueConnectionCount() { + reentrantLock.lock(); + try { + return claimedOverdueConnectionCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getAverageOverdueCheckoutTime() { - return claimedOverdueConnectionCount == 0 ? 0 - : accumulatedCheckoutTimeOfOverdueConnections / claimedOverdueConnectionCount; + public long getAverageOverdueCheckoutTime() { + reentrantLock.lock(); + try { + return claimedOverdueConnectionCount == 0 ? 0 + : accumulatedCheckoutTimeOfOverdueConnections / claimedOverdueConnectionCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized long getAverageCheckoutTime() { - return requestCount == 0 ? 0 : accumulatedCheckoutTime / requestCount; + public long getAverageCheckoutTime() { + reentrantLock.lock(); + try { + return requestCount == 0 ? 0 : accumulatedCheckoutTime / requestCount; + } finally { + reentrantLock.unlock(); + } } - public synchronized int getIdleConnectionCount() { - return idleConnections.size(); + public int getIdleConnectionCount() { + reentrantLock.lock(); + try { + return idleConnections.size(); + } finally { + reentrantLock.unlock(); + } } - public synchronized int getActiveConnectionCount() { - return activeConnections.size(); + public int getActiveConnectionCount() { + reentrantLock.lock(); + try { + return activeConnections.size(); + } finally { + reentrantLock.unlock(); + } } @Override - public synchronized String toString() { - StringBuilder builder = new StringBuilder(); - builder.append("\n===CONFIGURATION=============================================="); - builder.append("\n jdbcDriver ").append(dataSource.getDriver()); - builder.append("\n jdbcUrl ").append(dataSource.getUrl()); - builder.append("\n jdbcUsername ").append(dataSource.getUsername()); - builder.append("\n jdbcPassword ") - .append(dataSource.getPassword() == null ? "NULL" : "************"); - builder.append("\n poolMaxActiveConnections ").append(dataSource.poolMaximumActiveConnections); - builder.append("\n poolMaxIdleConnections ").append(dataSource.poolMaximumIdleConnections); - builder.append("\n poolMaxCheckoutTime ").append(dataSource.poolMaximumCheckoutTime); - builder.append("\n poolTimeToWait ").append(dataSource.poolTimeToWait); - builder.append("\n poolPingEnabled ").append(dataSource.poolPingEnabled); - builder.append("\n poolPingQuery ").append(dataSource.poolPingQuery); - builder.append("\n poolPingConnectionsNotUsedFor ").append(dataSource.poolPingConnectionsNotUsedFor); - builder.append("\n ---STATUS-----------------------------------------------------"); - builder.append("\n activeConnections ").append(getActiveConnectionCount()); - builder.append("\n idleConnections ").append(getIdleConnectionCount()); - builder.append("\n requestCount ").append(getRequestCount()); - builder.append("\n averageRequestTime ").append(getAverageRequestTime()); - builder.append("\n averageCheckoutTime ").append(getAverageCheckoutTime()); - builder.append("\n claimedOverdue ").append(getClaimedOverdueConnectionCount()); - builder.append("\n averageOverdueCheckoutTime ").append(getAverageOverdueCheckoutTime()); - builder.append("\n hadToWait ").append(getHadToWaitCount()); - builder.append("\n averageWaitTime ").append(getAverageWaitTime()); - builder.append("\n badConnectionCount ").append(getBadConnectionCount()); - builder.append("\n==============================================================="); - return builder.toString(); + public String toString() { + reentrantLock.lock(); + try { + StringBuilder builder = new StringBuilder(); + builder.append("\n===CONFIGURATION=============================================="); + builder.append("\n jdbcDriver ").append(dataSource.getDriver()); + builder.append("\n jdbcUrl ").append(dataSource.getUrl()); + builder.append("\n jdbcUsername ").append(dataSource.getUsername()); + builder.append("\n jdbcPassword ") + .append(dataSource.getPassword() == null ? "NULL" : "************"); + builder.append("\n poolMaxActiveConnections ").append(dataSource.poolMaximumActiveConnections); + builder.append("\n poolMaxIdleConnections ").append(dataSource.poolMaximumIdleConnections); + builder.append("\n poolMaxCheckoutTime ").append(dataSource.poolMaximumCheckoutTime); + builder.append("\n poolTimeToWait ").append(dataSource.poolTimeToWait); + builder.append("\n poolPingEnabled ").append(dataSource.poolPingEnabled); + builder.append("\n poolPingQuery ").append(dataSource.poolPingQuery); + builder.append("\n poolPingConnectionsNotUsedFor ").append(dataSource.poolPingConnectionsNotUsedFor); + builder.append("\n ---STATUS-----------------------------------------------------"); + builder.append("\n activeConnections ").append(getActiveConnectionCount()); + builder.append("\n idleConnections ").append(getIdleConnectionCount()); + builder.append("\n requestCount ").append(getRequestCount()); + builder.append("\n averageRequestTime ").append(getAverageRequestTime()); + builder.append("\n averageCheckoutTime ").append(getAverageCheckoutTime()); + builder.append("\n claimedOverdue ").append(getClaimedOverdueConnectionCount()); + builder.append("\n averageOverdueCheckoutTime ").append(getAverageOverdueCheckoutTime()); + builder.append("\n hadToWait ").append(getHadToWaitCount()); + builder.append("\n averageWaitTime ").append(getAverageWaitTime()); + builder.append("\n badConnectionCount ").append(getBadConnectionCount()); + builder.append("\n==============================================================="); + return builder.toString(); + } finally { + reentrantLock.unlock(); + } } } diff --git a/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java b/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java index becb88e2a58..b040ade7008 100644 --- a/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java +++ b/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java @@ -26,6 +26,7 @@ import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Logger; import javax.sql.DataSource; @@ -41,6 +42,7 @@ public class UnpooledDataSource implements DataSource { private ClassLoader driverClassLoader; private Properties driverProperties; private static final Map registeredDrivers = new ConcurrentHashMap<>(); + private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private String driver; private String url; @@ -137,12 +139,24 @@ public void setDriverProperties(Properties driverProperties) { this.driverProperties = driverProperties; } - public synchronized String getDriver() { - return driver; + public String getDriver() { + readWriteLock.readLock().lock(); + try { + return driver; + } finally { + readWriteLock.readLock().unlock(); + } + } - public synchronized void setDriver(String driver) { - this.driver = driver; + public void setDriver(String driver) { + readWriteLock.writeLock().lock(); + try { + this.driver = driver; + } finally { + readWriteLock.writeLock().unlock(); + } + } public String getUrl() { @@ -230,9 +244,10 @@ private Connection doGetConnection(Properties properties) throws SQLException { return connection; } - private synchronized void initializeDriver() throws SQLException { + private void initializeDriver() throws SQLException { if (!registeredDrivers.containsKey(driver)) { Class driverType; + readWriteLock.readLock().lock(); try { if (driverClassLoader != null) { driverType = Class.forName(driver, true, driverClassLoader); @@ -246,6 +261,8 @@ private synchronized void initializeDriver() throws SQLException { registeredDrivers.put(driver, driverInstance); } catch (Exception e) { throw new SQLException("Error setting driver on UnpooledDataSource. Cause: " + e); + } finally { + readWriteLock.readLock().lock(); } } } diff --git a/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java b/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java index ae48b53ceb2..97302e7e795 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java +++ b/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java @@ -25,6 +25,7 @@ import org.apache.ibatis.reflection.factory.ObjectFactory; import org.apache.ibatis.reflection.property.PropertyCopier; import org.apache.ibatis.reflection.property.PropertyNamer; +import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -38,7 +39,7 @@ public abstract class AbstractEnhancedDeserializationProxy { private final ObjectFactory objectFactory; private final List> constructorArgTypes; private final List constructorArgs; - private final Object reloadingPropertyLock; + private final LockKit.ReentrantLock reentrantLock; private boolean reloadingProperty; protected AbstractEnhancedDeserializationProxy(Class type, @@ -49,7 +50,7 @@ protected AbstractEnhancedDeserializationProxy(Class type, this.objectFactory = objectFactory; this.constructorArgTypes = constructorArgTypes; this.constructorArgs = constructorArgs; - this.reloadingPropertyLock = new Object(); + this.reentrantLock = new LockKit.ReentrantLock(); this.reloadingProperty = false; } @@ -68,7 +69,8 @@ public final Object invoke(Object enhanced, Method method, Object[] args) throws return this.newSerialStateHolder(original, unloadedProperties, objectFactory, constructorArgTypes, constructorArgs); } - synchronized (this.reloadingPropertyLock) { + reentrantLock.lock(); + try { if (!FINALIZE_METHOD.equals(methodName) && PropertyNamer.isProperty(methodName) && !reloadingProperty) { final String property = PropertyNamer.methodToProperty(methodName); final String propertyKey = property.toUpperCase(Locale.ENGLISH); @@ -93,6 +95,8 @@ public final Object invoke(Object enhanced, Method method, Object[] args) throws } return enhanced; + } finally { + reentrantLock.unlock(); } } catch (Throwable t) { throw ExceptionUtil.unwrapThrowable(t); diff --git a/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java b/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java index 3168ebaed67..29ea92f4684 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java +++ b/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java @@ -38,6 +38,7 @@ import org.apache.ibatis.reflection.property.PropertyCopier; import org.apache.ibatis.reflection.property.PropertyNamer; import org.apache.ibatis.session.Configuration; +import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -134,40 +135,42 @@ public static Object createProxy(Object target, ResultLoaderMap lazyLoader, Conf @Override public Object intercept(Object enhanced, Method method, Object[] args, MethodProxy methodProxy) throws Throwable { final String methodName = method.getName(); + LockKit.ReentrantLock lock = LockKit.obtainLock(lazyLoader); + lock.lock(); try { - synchronized (lazyLoader) { - if (WRITE_REPLACE_METHOD.equals(methodName)) { - Object original; - if (constructorArgTypes.isEmpty()) { - original = objectFactory.create(type); - } else { - original = objectFactory.create(type, constructorArgTypes, constructorArgs); - } - PropertyCopier.copyBeanProperties(type, enhanced, original); - if (lazyLoader.size() > 0) { - return new CglibSerialStateHolder(original, lazyLoader.getProperties(), objectFactory, - constructorArgTypes, constructorArgs); - } else { - return original; - } + if (WRITE_REPLACE_METHOD.equals(methodName)) { + Object original; + if (constructorArgTypes.isEmpty()) { + original = objectFactory.create(type); + } else { + original = objectFactory.create(type, constructorArgTypes, constructorArgs); } - if (lazyLoader.size() > 0 && !FINALIZE_METHOD.equals(methodName)) { - if (aggressive || lazyLoadTriggerMethods.contains(methodName)) { - lazyLoader.loadAll(); - } else if (PropertyNamer.isSetter(methodName)) { - final String property = PropertyNamer.methodToProperty(methodName); - lazyLoader.remove(property); - } else if (PropertyNamer.isGetter(methodName)) { - final String property = PropertyNamer.methodToProperty(methodName); - if (lazyLoader.hasLoader(property)) { - lazyLoader.load(property); - } + PropertyCopier.copyBeanProperties(type, enhanced, original); + if (lazyLoader.size() > 0) { + return new CglibSerialStateHolder(original, lazyLoader.getProperties(), objectFactory, constructorArgTypes, + constructorArgs); + } else { + return original; + } + } + if (lazyLoader.size() > 0 && !FINALIZE_METHOD.equals(methodName)) { + if (aggressive || lazyLoadTriggerMethods.contains(methodName)) { + lazyLoader.loadAll(); + } else if (PropertyNamer.isSetter(methodName)) { + final String property = PropertyNamer.methodToProperty(methodName); + lazyLoader.remove(property); + } else if (PropertyNamer.isGetter(methodName)) { + final String property = PropertyNamer.methodToProperty(methodName); + if (lazyLoader.hasLoader(property)) { + lazyLoader.load(property); } } } return methodProxy.invokeSuper(enhanced, args); } catch (Throwable t) { throw ExceptionUtil.unwrapThrowable(t); + } finally { + lock.unlock(); } } } diff --git a/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java b/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java index cf8c307a330..e40df18ede9 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java +++ b/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import javassist.util.proxy.MethodHandler; import javassist.util.proxy.Proxy; @@ -99,7 +100,7 @@ static Object createStaticProxy(Class type, MethodHandler callback, List type; private final ResultLoaderMap lazyLoader; private final boolean aggressive; @@ -132,40 +133,41 @@ public static Object createProxy(Object target, ResultLoaderMap lazyLoader, Conf @Override public Object invoke(Object enhanced, Method method, Method methodProxy, Object[] args) throws Throwable { final String methodName = method.getName(); + reentrantLock.lock(); try { - synchronized (lazyLoader) { - if (WRITE_REPLACE_METHOD.equals(methodName)) { - Object original; - if (constructorArgTypes.isEmpty()) { - original = objectFactory.create(type); - } else { - original = objectFactory.create(type, constructorArgTypes, constructorArgs); - } - PropertyCopier.copyBeanProperties(type, enhanced, original); - if (lazyLoader.size() > 0) { - return new JavassistSerialStateHolder(original, lazyLoader.getProperties(), objectFactory, - constructorArgTypes, constructorArgs); - } else { - return original; - } + if (WRITE_REPLACE_METHOD.equals(methodName)) { + Object original; + if (constructorArgTypes.isEmpty()) { + original = objectFactory.create(type); + } else { + original = objectFactory.create(type, constructorArgTypes, constructorArgs); } - if (lazyLoader.size() > 0 && !FINALIZE_METHOD.equals(methodName)) { - if (aggressive || lazyLoadTriggerMethods.contains(methodName)) { - lazyLoader.loadAll(); - } else if (PropertyNamer.isSetter(methodName)) { - final String property = PropertyNamer.methodToProperty(methodName); - lazyLoader.remove(property); - } else if (PropertyNamer.isGetter(methodName)) { - final String property = PropertyNamer.methodToProperty(methodName); - if (lazyLoader.hasLoader(property)) { - lazyLoader.load(property); - } + PropertyCopier.copyBeanProperties(type, enhanced, original); + if (lazyLoader.size() > 0) { + return new JavassistSerialStateHolder(original, lazyLoader.getProperties(), objectFactory, + constructorArgTypes, constructorArgs); + } else { + return original; + } + } + if (lazyLoader.size() > 0 && !FINALIZE_METHOD.equals(methodName)) { + if (aggressive || lazyLoadTriggerMethods.contains(methodName)) { + lazyLoader.loadAll(); + } else if (PropertyNamer.isSetter(methodName)) { + final String property = PropertyNamer.methodToProperty(methodName); + lazyLoader.remove(property); + } else if (PropertyNamer.isGetter(methodName)) { + final String property = PropertyNamer.methodToProperty(methodName); + if (lazyLoader.hasLoader(property)) { + lazyLoader.load(property); } } } return methodProxy.invoke(enhanced, args); } catch (Throwable t) { throw ExceptionUtil.unwrapThrowable(t); + } finally { + reentrantLock.unlock(); } } } diff --git a/src/main/java/org/apache/ibatis/io/JBoss6VFS.java b/src/main/java/org/apache/ibatis/io/JBoss6VFS.java index 0c56817364c..16b97e5d1e2 100644 --- a/src/main/java/org/apache/ibatis/io/JBoss6VFS.java +++ b/src/main/java/org/apache/ibatis/io/JBoss6VFS.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.locks.ReentrantLock; import org.apache.ibatis.logging.Log; import org.apache.ibatis.logging.LogFactory; @@ -32,6 +33,7 @@ */ public class JBoss6VFS extends VFS { private static final Log log = LogFactory.getLog(JBoss6VFS.class); + private static final ReentrantLock reentrantLock = new ReentrantLock(); /** A class that mimics a tiny subset of the JBoss VirtualFile class. */ static class VirtualFile { @@ -84,25 +86,30 @@ static VirtualFile getChild(URL url) throws IOException { private static Boolean valid; /** Find all the classes and methods that are required to access the JBoss 6 VFS. */ - protected static synchronized void initialize() { - if (valid == null) { - // Assume valid. It will get flipped later if something goes wrong. - valid = Boolean.TRUE; - - // Look up and verify required classes - VFS.VFS = checkNotNull(getClass("org.jboss.vfs.VFS")); - VirtualFile.VirtualFile = checkNotNull(getClass("org.jboss.vfs.VirtualFile")); - - // Look up and verify required methods - VFS.getChild = checkNotNull(getMethod(VFS.VFS, "getChild", URL.class)); - VirtualFile.getChildrenRecursively = checkNotNull(getMethod(VirtualFile.VirtualFile, "getChildrenRecursively")); - VirtualFile.getPathNameRelativeTo = checkNotNull( - getMethod(VirtualFile.VirtualFile, "getPathNameRelativeTo", VirtualFile.VirtualFile)); - - // Verify that the API has not changed - checkReturnType(VFS.getChild, VirtualFile.VirtualFile); - checkReturnType(VirtualFile.getChildrenRecursively, List.class); - checkReturnType(VirtualFile.getPathNameRelativeTo, String.class); + protected static void initialize() { + reentrantLock.lock(); + try { + if (valid == null) { + // Assume valid. It will get flipped later if something goes wrong. + valid = Boolean.TRUE; + + // Look up and verify required classes + VFS.VFS = checkNotNull(getClass("org.jboss.vfs.VFS")); + VirtualFile.VirtualFile = checkNotNull(getClass("org.jboss.vfs.VirtualFile")); + + // Look up and verify required methods + VFS.getChild = checkNotNull(getMethod(VFS.VFS, "getChild", URL.class)); + VirtualFile.getChildrenRecursively = checkNotNull(getMethod(VirtualFile.VirtualFile, "getChildrenRecursively")); + VirtualFile.getPathNameRelativeTo = checkNotNull( + getMethod(VirtualFile.VirtualFile, "getPathNameRelativeTo", VirtualFile.VirtualFile)); + + // Verify that the API has not changed + checkReturnType(VFS.getChild, VirtualFile.VirtualFile); + checkReturnType(VirtualFile.getChildrenRecursively, List.class); + checkReturnType(VirtualFile.getPathNameRelativeTo, String.class); + } + } finally { + reentrantLock.unlock(); } } diff --git a/src/main/java/org/apache/ibatis/logging/LogFactory.java b/src/main/java/org/apache/ibatis/logging/LogFactory.java index 229cc2901d1..dbb87fab4f6 100644 --- a/src/main/java/org/apache/ibatis/logging/LogFactory.java +++ b/src/main/java/org/apache/ibatis/logging/LogFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2022 the original author or authors. + * Copyright 2009-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.apache.ibatis.logging; import java.lang.reflect.Constructor; +import java.util.concurrent.locks.ReentrantLock; /** * @author Clinton Begin @@ -28,6 +29,7 @@ public final class LogFactory { */ public static final String MARKER = "MYBATIS"; + private static final ReentrantLock reentrantLock = new ReentrantLock(); private static Constructor logConstructor; static { @@ -55,15 +57,15 @@ public static Log getLog(String logger) { } } - public static synchronized void useCustomLogging(Class clazz) { + public static void useCustomLogging(Class clazz) { setImplementation(clazz); } - public static synchronized void useSlf4jLogging() { + public static void useSlf4jLogging() { setImplementation(org.apache.ibatis.logging.slf4j.Slf4jImpl.class); } - public static synchronized void useCommonsLogging() { + public static void useCommonsLogging() { setImplementation(org.apache.ibatis.logging.commons.JakartaCommonsLoggingImpl.class); } @@ -71,23 +73,23 @@ public static synchronized void useCommonsLogging() { * @deprecated Since 3.5.9 - See https://github.com/mybatis/mybatis-3/issues/1223. This method will remove future. */ @Deprecated - public static synchronized void useLog4JLogging() { + public static void useLog4JLogging() { setImplementation(org.apache.ibatis.logging.log4j.Log4jImpl.class); } - public static synchronized void useLog4J2Logging() { + public static void useLog4J2Logging() { setImplementation(org.apache.ibatis.logging.log4j2.Log4j2Impl.class); } - public static synchronized void useJdkLogging() { + public static void useJdkLogging() { setImplementation(org.apache.ibatis.logging.jdk14.Jdk14LoggingImpl.class); } - public static synchronized void useStdOutLogging() { + public static void useStdOutLogging() { setImplementation(org.apache.ibatis.logging.stdout.StdOutImpl.class); } - public static synchronized void useNoLogging() { + public static void useNoLogging() { setImplementation(org.apache.ibatis.logging.nologging.NoLoggingImpl.class); } @@ -102,6 +104,7 @@ private static void tryImplementation(Runnable runnable) { } private static void setImplementation(Class implClass) { + reentrantLock.lock(); try { Constructor candidate = implClass.getConstructor(String.class); Log log = candidate.newInstance(LogFactory.class.getName()); @@ -111,6 +114,8 @@ private static void setImplementation(Class implClass) { logConstructor = candidate; } catch (Throwable t) { throw new LogException("Error setting Log implementation. Cause: " + t, t); + } finally { + reentrantLock.unlock(); } } diff --git a/src/main/java/org/apache/ibatis/session/Configuration.java b/src/main/java/org/apache/ibatis/session/Configuration.java index cca5b8a0fea..dab3765c564 100644 --- a/src/main/java/org/apache/ibatis/session/Configuration.java +++ b/src/main/java/org/apache/ibatis/session/Configuration.java @@ -95,6 +95,7 @@ import org.apache.ibatis.type.TypeAliasRegistry; import org.apache.ibatis.type.TypeHandler; import org.apache.ibatis.type.TypeHandlerRegistry; +import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -166,7 +167,6 @@ public class Configuration { protected final Set loadedResources = new HashSet<>(); protected final Map sqlFragments = new StrictMap<>("XML fragments parsed from previous mappers"); - protected final Collection incompleteStatements = new LinkedList<>(); protected final Collection incompleteCacheRefs = new LinkedList<>(); protected final Collection incompleteResultMaps = new LinkedList<>(); @@ -925,24 +925,36 @@ public void addCacheRef(String namespace, String referencedNamespace) { protected void buildAllStatements() { parsePendingResultMaps(); if (!incompleteCacheRefs.isEmpty()) { - synchronized (incompleteCacheRefs) { + LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteCacheRefs); + lock.lock(); + try { incompleteCacheRefs.removeIf(x -> x.resolveCacheRef() != null); + } finally { + lock.unlock(); } } if (!incompleteStatements.isEmpty()) { - synchronized (incompleteStatements) { + LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteStatements); + lock.lock(); + try { incompleteStatements.removeIf(x -> { x.parseStatementNode(); return true; }); + } finally { + lock.unlock(); } } if (!incompleteMethods.isEmpty()) { - synchronized (incompleteMethods) { + LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteMethods); + lock.lock(); + try { incompleteMethods.removeIf(x -> { x.resolve(); return true; }); + } finally { + lock.unlock(); } } } @@ -951,7 +963,9 @@ private void parsePendingResultMaps() { if (incompleteResultMaps.isEmpty()) { return; } - synchronized (incompleteResultMaps) { + LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteResultMaps); + lock.lock(); + try { boolean resolved; IncompleteElementException ex = null; do { @@ -971,6 +985,8 @@ private void parsePendingResultMaps() { // At least one result map is unresolvable. throw ex; } + } finally { + lock.unlock(); } } diff --git a/src/main/java/org/apache/ibatis/util/LockKit.java b/src/main/java/org/apache/ibatis/util/LockKit.java new file mode 100644 index 00000000000..b828586e2d2 --- /dev/null +++ b/src/main/java/org/apache/ibatis/util/LockKit.java @@ -0,0 +1,113 @@ +/* + * Copyright 2009-2023 the original author or authors. + * + * 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 + * + * https://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.apache.ibatis.util; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.function.Supplier; + +/** + * This provides reentrant locking support for our code base. Future worlds like Loom virtual threads don't like + * synchronised calls since they pin the VT to the carrier thread. Word on the street is that locks are preferred to + * synchronised. + */ + +public class LockKit { + + private static final Map locks = new ConcurrentHashMap<>(); + + /** + * obtain a reentrant lock + * + * @param holder + * + * @return + */ + public static ReentrantLock obtainLock(Object holder) { + return locks.computeIfAbsent(holder, (key) -> new ReentrantLock()); + } + + /** + * A class to run code inside a reentrant lock + */ + public static class ReentrantLock { + private final Lock lock = new java.util.concurrent.locks.ReentrantLock(); + + /** + * Sometimes you need to directly lock things like for checked exceptions + *

+ * It's on you to unlock it! + */ + public void lock() { + lock.lock(); + } + + public void unlock() { + lock.unlock(); + } + + public void runLocked(Runnable codeToRun) { + lock.lock(); + try { + codeToRun.run(); + } finally { + lock.unlock(); + } + } + + public E callLocked(Supplier codeToRun) { + lock.lock(); + try { + return codeToRun.get(); + } finally { + lock.unlock(); + } + } + } + + /** + * Will allow for lazy computation of some values just once + */ + public static class ComputedOnce { + + private volatile boolean beenComputed = false; + private final ReentrantLock lock = new ReentrantLock(); + + public boolean hasBeenComputed() { + return beenComputed; + } + + public void runOnce(Runnable codeToRunOnce) { + if (beenComputed) { + return; + } + lock.runLocked(() -> { + // double lock check + if (beenComputed) { + return; + } + try { + codeToRunOnce.run(); + beenComputed = true; + } finally { + // even for exceptions we will say its computed + beenComputed = true; + } + }); + } + } +} diff --git a/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java b/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java index 5c92502ae19..e38cff5f359 100644 --- a/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java +++ b/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java @@ -21,24 +21,27 @@ import org.apache.ibatis.session.SqlSession; import org.apache.ibatis.session.SqlSessionFactory; import org.apache.ibatis.session.SqlSessionFactoryBuilder; +import org.apache.ibatis.util.LockKit; public class IbatisConfig { - + private static final LockKit.ComputedOnce computedOnce = new LockKit.ComputedOnce(); private static SqlSessionFactory sqlSessionFactory; private IbatisConfig() { } - private static synchronized void init() { - if (sqlSessionFactory == null) { - try { - final String resource = "org/apache/ibatis/submitted/primitive_result_type/ibatis.xml"; - Reader reader = Resources.getResourceAsReader(resource); - sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); - } catch (Exception e) { - throw new RuntimeException(e); + private static void init() { + computedOnce.runOnce(() -> { + if (sqlSessionFactory == null) { + try { + final String resource = "org/apache/ibatis/submitted/primitive_result_type/ibatis.xml"; + Reader reader = Resources.getResourceAsReader(resource); + sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); + } catch (Exception e) { + throw new RuntimeException(e); + } } - } + }); } public static SqlSession getSession() { From 4d704355c1793fd0dd27832f2b134e36e8365876 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sat, 10 Feb 2024 23:15:19 +0900 Subject: [PATCH 2/8] Reverting as this is just a test This test has unusual structure. If we want to remove `synchronized`, we should be able to rewrite the test without using `synchronized` like other tests. --- .../primitive_result_type/IbatisConfig.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java b/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java index e38cff5f359..2752f9067b8 100644 --- a/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java +++ b/src/test/java/org/apache/ibatis/submitted/primitive_result_type/IbatisConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,27 +21,24 @@ import org.apache.ibatis.session.SqlSession; import org.apache.ibatis.session.SqlSessionFactory; import org.apache.ibatis.session.SqlSessionFactoryBuilder; -import org.apache.ibatis.util.LockKit; public class IbatisConfig { - private static final LockKit.ComputedOnce computedOnce = new LockKit.ComputedOnce(); + private static SqlSessionFactory sqlSessionFactory; private IbatisConfig() { } - private static void init() { - computedOnce.runOnce(() -> { - if (sqlSessionFactory == null) { - try { - final String resource = "org/apache/ibatis/submitted/primitive_result_type/ibatis.xml"; - Reader reader = Resources.getResourceAsReader(resource); - sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); - } catch (Exception e) { - throw new RuntimeException(e); - } + private static synchronized void init() { + if (sqlSessionFactory == null) { + try { + final String resource = "org/apache/ibatis/submitted/primitive_result_type/ibatis.xml"; + Reader reader = Resources.getResourceAsReader(resource); + sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader); + } catch (Exception e) { + throw new RuntimeException(e); } - }); + } } public static SqlSession getSession() { From 50060a6daef284bec6330efed2de1f992e9b58cb Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:06:09 +0900 Subject: [PATCH 3/8] Use the `java.util.concurrent.locks.ReentrantLock` directly --- .../ibatis/cache/decorators/SoftCache.java | 7 +-- .../ibatis/cache/decorators/WeakCache.java | 7 +-- .../ibatis/datasource/pooled/PoolState.java | 57 ++++++++++--------- .../AbstractEnhancedDeserializationProxy.java | 11 ++-- .../loader/cglib/CglibProxyFactory.java | 6 +- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java b/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java index 79f0877b9d7..271bbe4a224 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/SoftCache.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,9 +19,9 @@ import java.lang.ref.SoftReference; import java.util.Deque; import java.util.LinkedList; +import java.util.concurrent.locks.ReentrantLock; import org.apache.ibatis.cache.Cache; -import org.apache.ibatis.util.LockKit; /** * Soft Reference cache decorator. @@ -35,6 +35,7 @@ public class SoftCache implements Cache { private final ReferenceQueue queueOfGarbageCollectedEntries; private final Cache delegate; private int numberOfHardLinks; + private final ReentrantLock lock = new ReentrantLock(); public SoftCache(Cache delegate) { this.delegate = delegate; @@ -75,7 +76,6 @@ public Object getObject(Object key) { delegate.removeObject(key); } else { // See #586 (and #335) modifications need more than a read lock - LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); lock.lock(); try { hardLinksToAvoidGarbageCollection.addFirst(result); @@ -100,7 +100,6 @@ public Object removeObject(Object key) { @Override public void clear() { - LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); lock.lock(); try { hardLinksToAvoidGarbageCollection.clear(); diff --git a/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java b/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java index 2c0964f5b27..772cdbafbed 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/WeakCache.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,9 +19,9 @@ import java.lang.ref.WeakReference; import java.util.Deque; import java.util.LinkedList; +import java.util.concurrent.locks.ReentrantLock; import org.apache.ibatis.cache.Cache; -import org.apache.ibatis.util.LockKit; /** * Weak Reference cache decorator. @@ -35,6 +35,7 @@ public class WeakCache implements Cache { private final ReferenceQueue queueOfGarbageCollectedEntries; private final Cache delegate; private int numberOfHardLinks; + private final ReentrantLock lock = new ReentrantLock(); public WeakCache(Cache delegate) { this.delegate = delegate; @@ -74,7 +75,6 @@ public Object getObject(Object key) { if (result == null) { delegate.removeObject(key); } else { - LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); lock.lock(); try { hardLinksToAvoidGarbageCollection.addFirst(result); @@ -99,7 +99,6 @@ public Object removeObject(Object key) { @Override public void clear() { - LockKit.ReentrantLock lock = LockKit.obtainLock(hardLinksToAvoidGarbageCollection); lock.lock(); try { hardLinksToAvoidGarbageCollection.clear(); diff --git a/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java b/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java index 4b7e6096ba0..bfd554c5a78 100644 --- a/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java +++ b/src/main/java/org/apache/ibatis/datasource/pooled/PoolState.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,15 +17,19 @@ import java.util.ArrayList; import java.util.List; - -import org.apache.ibatis.util.LockKit; +import java.util.concurrent.locks.ReentrantLock; /** * @author Clinton Begin */ public class PoolState { - private final LockKit.ReentrantLock reentrantLock; + // This lock does not guarantee consistency. + // Field values can be modified in PooledDataSource + // after the instance is returned from + // PooledDataSource#getPoolState(). + // A possible fix is to create and return a 'snapshot'. + private final ReentrantLock lock = new ReentrantLock(); protected PooledDataSource dataSource; @@ -42,103 +46,102 @@ public class PoolState { public PoolState(PooledDataSource dataSource) { this.dataSource = dataSource; - this.reentrantLock = LockKit.obtainLock(dataSource); } public long getRequestCount() { - reentrantLock.lock(); + lock.lock(); try { return requestCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getAverageRequestTime() { - reentrantLock.lock(); + lock.lock(); try { return requestCount == 0 ? 0 : accumulatedRequestTime / requestCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getAverageWaitTime() { - reentrantLock.lock(); + lock.lock(); try { return hadToWaitCount == 0 ? 0 : accumulatedWaitTime / hadToWaitCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getHadToWaitCount() { - reentrantLock.lock(); + lock.lock(); try { return hadToWaitCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getBadConnectionCount() { - reentrantLock.lock(); + lock.lock(); try { return badConnectionCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getClaimedOverdueConnectionCount() { - reentrantLock.lock(); + lock.lock(); try { return claimedOverdueConnectionCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getAverageOverdueCheckoutTime() { - reentrantLock.lock(); + lock.lock(); try { return claimedOverdueConnectionCount == 0 ? 0 : accumulatedCheckoutTimeOfOverdueConnections / claimedOverdueConnectionCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public long getAverageCheckoutTime() { - reentrantLock.lock(); + lock.lock(); try { return requestCount == 0 ? 0 : accumulatedCheckoutTime / requestCount; } finally { - reentrantLock.unlock(); + lock.unlock(); } } public int getIdleConnectionCount() { - reentrantLock.lock(); + lock.lock(); try { return idleConnections.size(); } finally { - reentrantLock.unlock(); + lock.unlock(); } } public int getActiveConnectionCount() { - reentrantLock.lock(); + lock.lock(); try { return activeConnections.size(); } finally { - reentrantLock.unlock(); + lock.unlock(); } } @Override public String toString() { - reentrantLock.lock(); + lock.lock(); try { StringBuilder builder = new StringBuilder(); builder.append("\n===CONFIGURATION=============================================="); @@ -168,7 +171,7 @@ public String toString() { builder.append("\n==============================================================="); return builder.toString(); } finally { - reentrantLock.unlock(); + lock.unlock(); } } diff --git a/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java b/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java index 97302e7e795..9e0032d836d 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java +++ b/src/main/java/org/apache/ibatis/executor/loader/AbstractEnhancedDeserializationProxy.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,13 +19,13 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.locks.ReentrantLock; import org.apache.ibatis.executor.ExecutorException; import org.apache.ibatis.reflection.ExceptionUtil; import org.apache.ibatis.reflection.factory.ObjectFactory; import org.apache.ibatis.reflection.property.PropertyCopier; import org.apache.ibatis.reflection.property.PropertyNamer; -import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -39,7 +39,7 @@ public abstract class AbstractEnhancedDeserializationProxy { private final ObjectFactory objectFactory; private final List> constructorArgTypes; private final List constructorArgs; - private final LockKit.ReentrantLock reentrantLock; + private final ReentrantLock lock = new ReentrantLock(); private boolean reloadingProperty; protected AbstractEnhancedDeserializationProxy(Class type, @@ -50,7 +50,6 @@ protected AbstractEnhancedDeserializationProxy(Class type, this.objectFactory = objectFactory; this.constructorArgTypes = constructorArgTypes; this.constructorArgs = constructorArgs; - this.reentrantLock = new LockKit.ReentrantLock(); this.reloadingProperty = false; } @@ -69,7 +68,7 @@ public final Object invoke(Object enhanced, Method method, Object[] args) throws return this.newSerialStateHolder(original, unloadedProperties, objectFactory, constructorArgTypes, constructorArgs); } - reentrantLock.lock(); + lock.lock(); try { if (!FINALIZE_METHOD.equals(methodName) && PropertyNamer.isProperty(methodName) && !reloadingProperty) { final String property = PropertyNamer.methodToProperty(methodName); @@ -96,7 +95,7 @@ public final Object invoke(Object enhanced, Method method, Object[] args) throws return enhanced; } finally { - reentrantLock.unlock(); + lock.unlock(); } } catch (Throwable t) { throw ExceptionUtil.unwrapThrowable(t); diff --git a/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java b/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java index 29ea92f4684..f027d8193a7 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java +++ b/src/main/java/org/apache/ibatis/executor/loader/cglib/CglibProxyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import net.sf.cglib.proxy.Callback; import net.sf.cglib.proxy.Enhancer; @@ -38,7 +39,6 @@ import org.apache.ibatis.reflection.property.PropertyCopier; import org.apache.ibatis.reflection.property.PropertyNamer; import org.apache.ibatis.session.Configuration; -import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -110,6 +110,7 @@ private static class EnhancedResultObjectProxyImpl implements MethodInterceptor private final ObjectFactory objectFactory; private final List> constructorArgTypes; private final List constructorArgs; + private final ReentrantLock lock = new ReentrantLock(); private EnhancedResultObjectProxyImpl(Class type, ResultLoaderMap lazyLoader, Configuration configuration, ObjectFactory objectFactory, List> constructorArgTypes, List constructorArgs) { @@ -135,7 +136,6 @@ public static Object createProxy(Object target, ResultLoaderMap lazyLoader, Conf @Override public Object intercept(Object enhanced, Method method, Object[] args, MethodProxy methodProxy) throws Throwable { final String methodName = method.getName(); - LockKit.ReentrantLock lock = LockKit.obtainLock(lazyLoader); lock.lock(); try { if (WRITE_REPLACE_METHOD.equals(methodName)) { From 69b382030ff6d7cd5e66b4ddd6e90e97ce4241c5 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:07:14 +0900 Subject: [PATCH 4/8] Rename the variable for consistency --- .../executor/loader/javassist/JavassistProxyFactory.java | 8 ++++---- src/main/java/org/apache/ibatis/io/JBoss6VFS.java | 8 ++++---- src/main/java/org/apache/ibatis/logging/LogFactory.java | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java b/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java index e40df18ede9..9bdb6d2fe1c 100644 --- a/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java +++ b/src/main/java/org/apache/ibatis/executor/loader/javassist/JavassistProxyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -100,7 +100,6 @@ static Object createStaticProxy(Class type, MethodHandler callback, List type; private final ResultLoaderMap lazyLoader; private final boolean aggressive; @@ -108,6 +107,7 @@ private static class EnhancedResultObjectProxyImpl implements MethodHandler { private final ObjectFactory objectFactory; private final List> constructorArgTypes; private final List constructorArgs; + private final ReentrantLock lock = new ReentrantLock(); private EnhancedResultObjectProxyImpl(Class type, ResultLoaderMap lazyLoader, Configuration configuration, ObjectFactory objectFactory, List> constructorArgTypes, List constructorArgs) { @@ -133,7 +133,7 @@ public static Object createProxy(Object target, ResultLoaderMap lazyLoader, Conf @Override public Object invoke(Object enhanced, Method method, Method methodProxy, Object[] args) throws Throwable { final String methodName = method.getName(); - reentrantLock.lock(); + lock.lock(); try { if (WRITE_REPLACE_METHOD.equals(methodName)) { Object original; @@ -167,7 +167,7 @@ public Object invoke(Object enhanced, Method method, Method methodProxy, Object[ } catch (Throwable t) { throw ExceptionUtil.unwrapThrowable(t); } finally { - reentrantLock.unlock(); + lock.unlock(); } } } diff --git a/src/main/java/org/apache/ibatis/io/JBoss6VFS.java b/src/main/java/org/apache/ibatis/io/JBoss6VFS.java index 16b97e5d1e2..bd2d68ad51c 100644 --- a/src/main/java/org/apache/ibatis/io/JBoss6VFS.java +++ b/src/main/java/org/apache/ibatis/io/JBoss6VFS.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,7 +33,7 @@ */ public class JBoss6VFS extends VFS { private static final Log log = LogFactory.getLog(JBoss6VFS.class); - private static final ReentrantLock reentrantLock = new ReentrantLock(); + private static final ReentrantLock lock = new ReentrantLock(); /** A class that mimics a tiny subset of the JBoss VirtualFile class. */ static class VirtualFile { @@ -87,7 +87,7 @@ static VirtualFile getChild(URL url) throws IOException { /** Find all the classes and methods that are required to access the JBoss 6 VFS. */ protected static void initialize() { - reentrantLock.lock(); + lock.lock(); try { if (valid == null) { // Assume valid. It will get flipped later if something goes wrong. @@ -109,7 +109,7 @@ protected static void initialize() { checkReturnType(VirtualFile.getPathNameRelativeTo, String.class); } } finally { - reentrantLock.unlock(); + lock.unlock(); } } diff --git a/src/main/java/org/apache/ibatis/logging/LogFactory.java b/src/main/java/org/apache/ibatis/logging/LogFactory.java index dbb87fab4f6..f70412b3459 100644 --- a/src/main/java/org/apache/ibatis/logging/LogFactory.java +++ b/src/main/java/org/apache/ibatis/logging/LogFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,7 +29,7 @@ public final class LogFactory { */ public static final String MARKER = "MYBATIS"; - private static final ReentrantLock reentrantLock = new ReentrantLock(); + private static final ReentrantLock lock = new ReentrantLock(); private static Constructor logConstructor; static { @@ -104,7 +104,7 @@ private static void tryImplementation(Runnable runnable) { } private static void setImplementation(Class implClass) { - reentrantLock.lock(); + lock.lock(); try { Constructor candidate = implClass.getConstructor(String.class); Log log = candidate.newInstance(LogFactory.class.getName()); @@ -115,7 +115,7 @@ private static void setImplementation(Class implClass) { } catch (Throwable t) { throw new LogException("Error setting Log implementation. Cause: " + t, t); } finally { - reentrantLock.unlock(); + lock.unlock(); } } From 79f33a4a9438a3a8c6d0d60e71e4dc694b1a5c0b Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:12:06 +0900 Subject: [PATCH 5/8] Use `computeIfAbsent` instead of explicit lock --- .../unpooled/UnpooledDataSource.java | 56 +++++++------------ 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java b/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java index b040ade7008..1b5c769d7c6 100644 --- a/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java +++ b/src/main/java/org/apache/ibatis/datasource/unpooled/UnpooledDataSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,12 +26,12 @@ import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; -import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Logger; import javax.sql.DataSource; import org.apache.ibatis.io.Resources; +import org.apache.ibatis.util.MapUtil; /** * @author Clinton Begin @@ -42,7 +42,6 @@ public class UnpooledDataSource implements DataSource { private ClassLoader driverClassLoader; private Properties driverProperties; private static final Map registeredDrivers = new ConcurrentHashMap<>(); - private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private String driver; private String url; @@ -140,23 +139,11 @@ public void setDriverProperties(Properties driverProperties) { } public String getDriver() { - readWriteLock.readLock().lock(); - try { - return driver; - } finally { - readWriteLock.readLock().unlock(); - } - + return driver; } public void setDriver(String driver) { - readWriteLock.writeLock().lock(); - try { - this.driver = driver; - } finally { - readWriteLock.writeLock().unlock(); - } - + this.driver = driver; } public String getUrl() { @@ -245,25 +232,24 @@ private Connection doGetConnection(Properties properties) throws SQLException { } private void initializeDriver() throws SQLException { - if (!registeredDrivers.containsKey(driver)) { - Class driverType; - readWriteLock.readLock().lock(); - try { - if (driverClassLoader != null) { - driverType = Class.forName(driver, true, driverClassLoader); - } else { - driverType = Resources.classForName(driver); + try { + MapUtil.computeIfAbsent(registeredDrivers, driver, x -> { + Class driverType; + try { + if (driverClassLoader != null) { + driverType = Class.forName(x, true, driverClassLoader); + } else { + driverType = Resources.classForName(x); + } + Driver driverInstance = (Driver) driverType.getDeclaredConstructor().newInstance(); + DriverManager.registerDriver(new DriverProxy(driverInstance)); + return driverInstance; + } catch (Exception e) { + throw new RuntimeException("Error setting driver on UnpooledDataSource.", e); } - // DriverManager requires the driver to be loaded via the system ClassLoader. - // https://www.kfu.com/~nsayer/Java/dyn-jdbc.html - Driver driverInstance = (Driver) driverType.getDeclaredConstructor().newInstance(); - DriverManager.registerDriver(new DriverProxy(driverInstance)); - registeredDrivers.put(driver, driverInstance); - } catch (Exception e) { - throw new SQLException("Error setting driver on UnpooledDataSource. Cause: " + e); - } finally { - readWriteLock.readLock().lock(); - } + }); + } catch (RuntimeException re) { + throw new SQLException("Error setting driver on UnpooledDataSource.", re.getCause()); } } From ac9d83661095c47cf289e4aef68e7946b923d58f Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:17:38 +0900 Subject: [PATCH 6/8] Use `ReentrantLock` instead of `ReentrantReadWriteLock` Using `ReentrantReadWriteLock` changes the behavior and it could increase the chance of cache stampede. --- .../cache/decorators/SynchronizedCache.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java b/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java index 5e8c7a7c9d9..13076bd4d60 100644 --- a/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java +++ b/src/main/java/org/apache/ibatis/cache/decorators/SynchronizedCache.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,7 +15,7 @@ */ package org.apache.ibatis.cache.decorators; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.ibatis.cache.Cache; @@ -24,7 +24,7 @@ */ public class SynchronizedCache implements Cache { - private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + private final ReentrantLock lock = new ReentrantLock(); private final Cache delegate; public SynchronizedCache(Cache delegate) { @@ -38,51 +38,51 @@ public String getId() { @Override public int getSize() { - readWriteLock.readLock().lock(); + lock.lock(); try { return delegate.getSize(); } finally { - readWriteLock.readLock().unlock(); + lock.unlock(); } } @Override public void putObject(Object key, Object object) { - readWriteLock.writeLock().lock(); + lock.lock(); try { delegate.putObject(key, object); } finally { - readWriteLock.writeLock().unlock(); + lock.unlock(); } } @Override public Object getObject(Object key) { - readWriteLock.readLock().lock(); + lock.lock(); try { return delegate.getObject(key); } finally { - readWriteLock.readLock().unlock(); + lock.unlock(); } } @Override public Object removeObject(Object key) { - readWriteLock.writeLock().lock(); + lock.lock(); try { return delegate.removeObject(key); } finally { - readWriteLock.writeLock().unlock(); + lock.unlock(); } } @Override public void clear() { - readWriteLock.writeLock().lock(); + lock.lock(); try { delegate.clear(); } finally { - readWriteLock.writeLock().unlock(); + lock.unlock(); } } From fba61d2b752a84cb439eb03ae56776da4a1d4af7 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:25:26 +0900 Subject: [PATCH 7/8] Refactoring : Parse pending elements in `Configuration` --- .../annotation/MapperAnnotationBuilder.java | 20 +-- .../ibatis/builder/xml/XMLMapperBuilder.java | 55 +------ .../apache/ibatis/session/Configuration.java | 142 +++++++++++++----- 3 files changed, 109 insertions(+), 108 deletions(-) diff --git a/src/main/java/org/apache/ibatis/builder/annotation/MapperAnnotationBuilder.java b/src/main/java/org/apache/ibatis/builder/annotation/MapperAnnotationBuilder.java index 61c62ad360d..0388256e5e7 100644 --- a/src/main/java/org/apache/ibatis/builder/annotation/MapperAnnotationBuilder.java +++ b/src/main/java/org/apache/ibatis/builder/annotation/MapperAnnotationBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -135,7 +134,7 @@ public void parse() { } } } - parsePendingMethods(); + configuration.parsePendingMethods(false); } private static boolean canHaveStatement(Method method) { @@ -143,21 +142,6 @@ private static boolean canHaveStatement(Method method) { return !method.isBridge() && !method.isDefault(); } - private void parsePendingMethods() { - Collection incompleteMethods = configuration.getIncompleteMethods(); - synchronized (incompleteMethods) { - Iterator iter = incompleteMethods.iterator(); - while (iter.hasNext()) { - try { - iter.next().resolve(); - iter.remove(); - } catch (IncompleteElementException e) { - // This method is still missing a resource - } - } - } - } - private void loadXmlResource() { // Spring may not know the real resource name so we check a flag // to prevent loading again a resource twice diff --git a/src/main/java/org/apache/ibatis/builder/xml/XMLMapperBuilder.java b/src/main/java/org/apache/ibatis/builder/xml/XMLMapperBuilder.java index 4c114537c4d..ea2577381e3 100644 --- a/src/main/java/org/apache/ibatis/builder/xml/XMLMapperBuilder.java +++ b/src/main/java/org/apache/ibatis/builder/xml/XMLMapperBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,10 +19,8 @@ import java.io.Reader; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Properties; @@ -101,9 +99,9 @@ public void parse() { configuration.addLoadedResource(resource); bindMapperForNamespace(); } - parsePendingResultMaps(); - parsePendingCacheRefs(); - parsePendingStatements(); + configuration.parsePendingResultMaps(false); + configuration.parsePendingCacheRefs(false); + configuration.parsePendingStatements(false); } public XNode getSqlFragment(String refid) { @@ -147,51 +145,6 @@ private void buildStatementFromContext(List list, String requiredDatabase } } - private void parsePendingResultMaps() { - Collection incompleteResultMaps = configuration.getIncompleteResultMaps(); - synchronized (incompleteResultMaps) { - Iterator iter = incompleteResultMaps.iterator(); - while (iter.hasNext()) { - try { - iter.next().resolve(); - iter.remove(); - } catch (IncompleteElementException e) { - // ResultMap is still missing a resource... - } - } - } - } - - private void parsePendingCacheRefs() { - Collection incompleteCacheRefs = configuration.getIncompleteCacheRefs(); - synchronized (incompleteCacheRefs) { - Iterator iter = incompleteCacheRefs.iterator(); - while (iter.hasNext()) { - try { - iter.next().resolveCacheRef(); - iter.remove(); - } catch (IncompleteElementException e) { - // Cache ref is still missing a resource... - } - } - } - } - - private void parsePendingStatements() { - Collection incompleteStatements = configuration.getIncompleteStatements(); - synchronized (incompleteStatements) { - Iterator iter = incompleteStatements.iterator(); - while (iter.hasNext()) { - try { - iter.next().parseStatementNode(); - iter.remove(); - } catch (IncompleteElementException e) { - // Statement is still missing a resource... - } - } - } - } - private void cacheRefElement(XNode context) { if (context != null) { configuration.addCacheRef(builderAssistant.getCurrentNamespace(), context.getStringAttribute("namespace")); diff --git a/src/main/java/org/apache/ibatis/session/Configuration.java b/src/main/java/org/apache/ibatis/session/Configuration.java index dab3765c564..72b50d3808f 100644 --- a/src/main/java/org/apache/ibatis/session/Configuration.java +++ b/src/main/java/org/apache/ibatis/session/Configuration.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2023 the original author or authors. + * Copyright 2009-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiFunction; import org.apache.ibatis.binding.MapperRegistry; @@ -95,7 +96,6 @@ import org.apache.ibatis.type.TypeAliasRegistry; import org.apache.ibatis.type.TypeHandler; import org.apache.ibatis.type.TypeHandlerRegistry; -import org.apache.ibatis.util.LockKit; /** * @author Clinton Begin @@ -172,6 +172,11 @@ public class Configuration { protected final Collection incompleteResultMaps = new LinkedList<>(); protected final Collection incompleteMethods = new LinkedList<>(); + private final ReentrantLock incompleteResultMapsLock = new ReentrantLock(); + private final ReentrantLock incompleteCacheRefsLock = new ReentrantLock(); + private final ReentrantLock incompleteStatementsLock = new ReentrantLock(); + private final ReentrantLock incompleteMethodsLock = new ReentrantLock(); + /* * A map holds cache-ref relationship. The key is the namespace that references a cache bound to another namespace and * the value is the namespace which the actual cache is bound to. @@ -832,34 +837,70 @@ public Collection getMappedStatements() { return mappedStatements.values(); } + /** + * @deprecated call {@link #parsePendingStatements(boolean)} + */ + @Deprecated public Collection getIncompleteStatements() { return incompleteStatements; } public void addIncompleteStatement(XMLStatementBuilder incompleteStatement) { - incompleteStatements.add(incompleteStatement); + incompleteStatementsLock.lock(); + try { + incompleteStatements.add(incompleteStatement); + } finally { + incompleteStatementsLock.unlock(); + } } + /** + * @deprecated call {@link #parsePendingCacheRefs(boolean)} + */ + @Deprecated public Collection getIncompleteCacheRefs() { return incompleteCacheRefs; } public void addIncompleteCacheRef(CacheRefResolver incompleteCacheRef) { - incompleteCacheRefs.add(incompleteCacheRef); + incompleteCacheRefsLock.lock(); + try { + incompleteCacheRefs.add(incompleteCacheRef); + } finally { + incompleteCacheRefsLock.unlock(); + } } + /** + * @deprecated call {@link #parsePendingResultMaps(boolean)} + */ + @Deprecated public Collection getIncompleteResultMaps() { return incompleteResultMaps; } public void addIncompleteResultMap(ResultMapResolver resultMapResolver) { - incompleteResultMaps.add(resultMapResolver); + incompleteResultMapsLock.lock(); + try { + incompleteResultMaps.add(resultMapResolver); + } finally { + incompleteResultMapsLock.unlock(); + } } public void addIncompleteMethod(MethodResolver builder) { - incompleteMethods.add(builder); + incompleteMethodsLock.lock(); + try { + incompleteMethods.add(builder); + } finally { + incompleteMethodsLock.unlock(); + } } + /** + * @deprecated call {@link #parsePendingMethods(boolean)} + */ + @Deprecated public Collection getIncompleteMethods() { return incompleteMethods; } @@ -923,48 +964,71 @@ public void addCacheRef(String namespace, String referencedNamespace) { * are added as it provides fail-fast statement validation. */ protected void buildAllStatements() { - parsePendingResultMaps(); - if (!incompleteCacheRefs.isEmpty()) { - LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteCacheRefs); - lock.lock(); - try { - incompleteCacheRefs.removeIf(x -> x.resolveCacheRef() != null); - } finally { - lock.unlock(); + parsePendingResultMaps(true); + parsePendingCacheRefs(true); + parsePendingStatements(true); + parsePendingMethods(true); + } + + public void parsePendingMethods(boolean reportUnresolved) { + if (incompleteMethods.isEmpty()) { + return; + } + incompleteMethodsLock.lock(); + try { + incompleteMethods.removeIf(x -> { + x.resolve(); + return true; + }); + } catch (IncompleteElementException e) { + if (reportUnresolved) { + throw e; } + } finally { + incompleteMethodsLock.unlock(); } - if (!incompleteStatements.isEmpty()) { - LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteStatements); - lock.lock(); - try { - incompleteStatements.removeIf(x -> { - x.parseStatementNode(); - return true; - }); - } finally { - lock.unlock(); + } + + public void parsePendingStatements(boolean reportUnresolved) { + if (incompleteStatements.isEmpty()) { + return; + } + incompleteStatementsLock.lock(); + try { + incompleteStatements.removeIf(x -> { + x.parseStatementNode(); + return true; + }); + } catch (IncompleteElementException e) { + if (reportUnresolved) { + throw e; } + } finally { + incompleteStatementsLock.unlock(); + } + } + + public void parsePendingCacheRefs(boolean reportUnresolved) { + if (incompleteCacheRefs.isEmpty()) { + return; } - if (!incompleteMethods.isEmpty()) { - LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteMethods); - lock.lock(); - try { - incompleteMethods.removeIf(x -> { - x.resolve(); - return true; - }); - } finally { - lock.unlock(); + incompleteCacheRefsLock.lock(); + try { + incompleteCacheRefs.removeIf(x -> x.resolveCacheRef() != null); + } catch (IncompleteElementException e) { + if (reportUnresolved) { + throw e; } + } finally { + incompleteCacheRefsLock.unlock(); } } - private void parsePendingResultMaps() { + public void parsePendingResultMaps(boolean reportUnresolved) { if (incompleteResultMaps.isEmpty()) { return; } - LockKit.ReentrantLock lock = LockKit.obtainLock(incompleteResultMaps); - lock.lock(); + incompleteResultMapsLock.lock(); try { boolean resolved; IncompleteElementException ex = null; @@ -981,12 +1045,12 @@ private void parsePendingResultMaps() { } } } while (resolved); - if (!incompleteResultMaps.isEmpty() && ex != null) { + if (reportUnresolved && !incompleteResultMaps.isEmpty() && ex != null) { // At least one result map is unresolvable. throw ex; } } finally { - lock.unlock(); + incompleteResultMapsLock.unlock(); } } From 0949fc70847e80a4d78a8c8f4cc71309b420a2a0 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 11 Feb 2024 08:25:36 +0900 Subject: [PATCH 8/8] Drop LockKit --- .../java/org/apache/ibatis/util/LockKit.java | 113 ------------------ 1 file changed, 113 deletions(-) delete mode 100644 src/main/java/org/apache/ibatis/util/LockKit.java diff --git a/src/main/java/org/apache/ibatis/util/LockKit.java b/src/main/java/org/apache/ibatis/util/LockKit.java deleted file mode 100644 index b828586e2d2..00000000000 --- a/src/main/java/org/apache/ibatis/util/LockKit.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright 2009-2023 the original author or authors. - * - * 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 - * - * https://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.apache.ibatis.util; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.Lock; -import java.util.function.Supplier; - -/** - * This provides reentrant locking support for our code base. Future worlds like Loom virtual threads don't like - * synchronised calls since they pin the VT to the carrier thread. Word on the street is that locks are preferred to - * synchronised. - */ - -public class LockKit { - - private static final Map locks = new ConcurrentHashMap<>(); - - /** - * obtain a reentrant lock - * - * @param holder - * - * @return - */ - public static ReentrantLock obtainLock(Object holder) { - return locks.computeIfAbsent(holder, (key) -> new ReentrantLock()); - } - - /** - * A class to run code inside a reentrant lock - */ - public static class ReentrantLock { - private final Lock lock = new java.util.concurrent.locks.ReentrantLock(); - - /** - * Sometimes you need to directly lock things like for checked exceptions - *

- * It's on you to unlock it! - */ - public void lock() { - lock.lock(); - } - - public void unlock() { - lock.unlock(); - } - - public void runLocked(Runnable codeToRun) { - lock.lock(); - try { - codeToRun.run(); - } finally { - lock.unlock(); - } - } - - public E callLocked(Supplier codeToRun) { - lock.lock(); - try { - return codeToRun.get(); - } finally { - lock.unlock(); - } - } - } - - /** - * Will allow for lazy computation of some values just once - */ - public static class ComputedOnce { - - private volatile boolean beenComputed = false; - private final ReentrantLock lock = new ReentrantLock(); - - public boolean hasBeenComputed() { - return beenComputed; - } - - public void runOnce(Runnable codeToRunOnce) { - if (beenComputed) { - return; - } - lock.runLocked(() -> { - // double lock check - if (beenComputed) { - return; - } - try { - codeToRunOnce.run(); - beenComputed = true; - } finally { - // even for exceptions we will say its computed - beenComputed = true; - } - }); - } - } -}