From 380cb706c8e043d21b36139019d482401dd42fa4 Mon Sep 17 00:00:00 2001 From: lukaseder Date: Tue, 16 Jan 2018 10:43:05 +0100 Subject: [PATCH] [#7062] NotSerializableException when Record references reflection cache --- .../org/jooq/impl/DefaultConfiguration.java | 63 ++++++++++++++----- .../impl/DefaultRecordMapperProvider.java | 2 +- jOOQ/src/main/java/org/jooq/impl/Tools.java | 36 +++++++---- 3 files changed, 71 insertions(+), 30 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java b/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java index dbad6f2bcc..3caccff44e 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java @@ -48,6 +48,7 @@ import java.io.StringWriter; import java.sql.Connection; import java.time.Clock; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; @@ -79,6 +80,7 @@ import org.jooq.conf.Settings; import org.jooq.conf.SettingsTools; import org.jooq.exception.ConfigurationException; import org.jooq.impl.ThreadLocalTransactionProvider.ThreadLocalConnectionProvider; +import org.jooq.impl.Tools.DataCacheKey; /** * A default implementation for configurations within a {@link DSLContext}, if no @@ -94,30 +96,34 @@ public class DefaultConfiguration implements Configuration { /** * Serial version UID */ - private static final long serialVersionUID = 8193158984283234708L; + private static final long serialVersionUID = 4296981040491238190L; // Configuration objects - private SQLDialect dialect; - private Settings settings; - private ConcurrentHashMap data; + private SQLDialect dialect; + private Settings settings; - private Clock clock; + private Clock clock; - // Non-serializable Configuration objects - private transient ConnectionProvider connectionProvider; - private transient ExecutorProvider executorProvider; - private transient TransactionProvider transactionProvider; - private transient RecordMapperProvider recordMapperProvider; - private transient RecordUnmapperProvider recordUnmapperProvider; - private transient RecordListenerProvider[] recordListenerProviders; - private transient ExecuteListenerProvider[] executeListenerProviders; - private transient VisitListenerProvider[] visitListenerProviders; - private transient TransactionListenerProvider[] transactionListenerProviders; - private transient ConverterProvider converterProvider; + // These objects may be user defined and thus not necessarily serialisable + private transient ConnectionProvider connectionProvider; + private transient ExecutorProvider executorProvider; + private transient TransactionProvider transactionProvider; + private transient RecordMapperProvider recordMapperProvider; + private transient RecordUnmapperProvider recordUnmapperProvider; + private transient RecordListenerProvider[] recordListenerProviders; + private transient ExecuteListenerProvider[] executeListenerProviders; + private transient VisitListenerProvider[] visitListenerProviders; + private transient TransactionListenerProvider[] transactionListenerProviders; + private transient ConverterProvider converterProvider; + + // [#7062] Apart from the possibility of containing user defined objects, the data + // map also contains the reflection cache, which isn't serializable (and + // should not be serialized anyway). + private transient ConcurrentHashMap data; // Derived objects - private org.jooq.SchemaMapping mapping; + private org.jooq.SchemaMapping mapping; // ------------------------------------------------------------------------- // XXX: Constructors @@ -1297,8 +1303,22 @@ public class DefaultConfiguration implements Configuration { oos.writeObject(converterProvider instanceof Serializable ? converterProvider : null); + + // [#7062] Exclude reflection cache from serialisation + for (Entry entry : data.entrySet()) { + if (entry.getKey() instanceof DataCacheKey) + continue; + + oos.writeObject(entry.getKey()); + oos.writeObject(entry.getValue()); + } + + // [#7062] End of Map marker + oos.writeObject(END_OF_MAP_MARKER); } + private static final String END_OF_MAP_MARKER = "EOM"; + private E[] cloneSerializables(E[] array) { E[] clone = array.clone(); @@ -1323,6 +1343,15 @@ public class DefaultConfiguration implements Configuration { visitListenerProviders = (VisitListenerProvider[]) ois.readObject(); transactionListenerProviders = (TransactionListenerProvider[]) ois.readObject(); converterProvider = (ConverterProvider) ois.readObject(); + data = new ConcurrentHashMap(); + + Object key; + Object value; + + while (!END_OF_MAP_MARKER.equals(key = ois.readObject())) { + value = ois.readObject(); + data.put(key, value); + } } private final class ExecutorWrapper implements ExecutorProvider { diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapperProvider.java b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapperProvider.java index f866d56e94..1ece21a3c0 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapperProvider.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapperProvider.java @@ -38,7 +38,7 @@ package org.jooq.impl; import static java.lang.Boolean.TRUE; -import static org.jooq.impl.Tools.DATA_CACHE_RECORD_MAPPERS; +import static org.jooq.impl.Tools.DataCacheKey.DATA_CACHE_RECORD_MAPPERS; import java.io.Serializable; diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 6ce05dd820..b881e11eed 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -129,6 +129,13 @@ import static org.jooq.impl.Keywords.K_THEN; import static org.jooq.impl.Keywords.K_THROW; import static org.jooq.impl.Keywords.K_WHEN; import static org.jooq.impl.SQLDataType.VARCHAR; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_ANNOTATED_GETTER; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_ANNOTATED_MEMBERS; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_ANNOTATED_SETTERS; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_MATCHING_GETTER; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_MATCHING_MEMBERS; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_GET_MATCHING_SETTERS; +import static org.jooq.impl.Tools.DataCacheKey.DATA_REFLECTION_CACHE_HAS_COLUMN_ANNOTATIONS; import static org.jooq.impl.Tools.DataKey.DATA_BLOCK_NESTING; import static org.jooq.tools.reflect.Reflect.accessible; @@ -469,14 +476,22 @@ final class Tools { * new String() is used to allow for synchronizing on these * objects. */ - static final String DATA_REFLECTION_CACHE_GET_ANNOTATED_GETTER = new String("org.jooq.configuration.reflection-cache.get-annotated-getter"); - static final String DATA_REFLECTION_CACHE_GET_ANNOTATED_MEMBERS = new String("org.jooq.configuration.reflection-cache.get-annotated-members"); - static final String DATA_REFLECTION_CACHE_GET_ANNOTATED_SETTERS = new String("org.jooq.configuration.reflection-cache.get-annotated-setters"); - static final String DATA_REFLECTION_CACHE_GET_MATCHING_GETTER = new String("org.jooq.configuration.reflection-cache.get-matching-getter"); - static final String DATA_REFLECTION_CACHE_GET_MATCHING_MEMBERS = new String("org.jooq.configuration.reflection-cache.get-matching-members"); - static final String DATA_REFLECTION_CACHE_GET_MATCHING_SETTERS = new String("org.jooq.configuration.reflection-cache.get-matching-setters"); - static final String DATA_REFLECTION_CACHE_HAS_COLUMN_ANNOTATIONS = new String("org.jooq.configuration.reflection-cache.has-column-annotations"); - static final String DATA_CACHE_RECORD_MAPPERS = new String("org.jooq.configuration.cache.record-mappers"); + enum DataCacheKey { + DATA_REFLECTION_CACHE_GET_ANNOTATED_GETTER("org.jooq.configuration.reflection-cache.get-annotated-getter"), + DATA_REFLECTION_CACHE_GET_ANNOTATED_MEMBERS("org.jooq.configuration.reflection-cache.get-annotated-members"), + DATA_REFLECTION_CACHE_GET_ANNOTATED_SETTERS("org.jooq.configuration.reflection-cache.get-annotated-setters"), + DATA_REFLECTION_CACHE_GET_MATCHING_GETTER("org.jooq.configuration.reflection-cache.get-matching-getter"), + DATA_REFLECTION_CACHE_GET_MATCHING_MEMBERS("org.jooq.configuration.reflection-cache.get-matching-members"), + DATA_REFLECTION_CACHE_GET_MATCHING_SETTERS("org.jooq.configuration.reflection-cache.get-matching-setters"), + DATA_REFLECTION_CACHE_HAS_COLUMN_ANNOTATIONS("org.jooq.configuration.reflection-cache.has-column-annotations"), + DATA_CACHE_RECORD_MAPPERS("org.jooq.configuration.cache.record-mappers"); + + final String key; + + private DataCacheKey(String key) { + this.key = key; + } + } // ------------------------------------------------------------------------ // Other constants @@ -2684,7 +2699,7 @@ final class Tools { * @return The cached value or the outcome of the cached operation. */ @SuppressWarnings("unchecked") - static final V run(Configuration configuration, CachedOperation operation, String type, Object key) { + static final V run(Configuration configuration, CachedOperation operation, DataCacheKey type, Object key) { // If no configuration is provided take the default configuration that loads the default Settings if (configuration == null) @@ -2696,8 +2711,6 @@ final class Tools { Map cache = (Map) configuration.data(type); if (cache == null) { - - // String synchronization is OK as all type literals were created using new String() synchronized (type) { cache = (Map) configuration.data(type); @@ -2709,7 +2722,6 @@ final class Tools { } Object result = cache.get(key); - if (result == null) { synchronized (cache) { result = cache.get(key);