Skip to content

Commit

Permalink
[#7062] NotSerializableException when Record references reflection cache
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukaseder
lukaseder committed Jan 16, 2018
1 parent 0187539 commit 380cb70
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 30 deletions.
63 changes: 46 additions & 17 deletions jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java
View file
Expand Up @@ -48,6 +48,7 @@
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;

Expand Down Expand Up @@ -79,6 +80,7 @@
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
Expand All @@ -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<Object, Object> 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<Object, Object> data;

// Derived objects
private org.jooq.SchemaMapping mapping;
private org.jooq.SchemaMapping mapping;

// -------------------------------------------------------------------------
// XXX: Constructors
Expand Down Expand Up @@ -1297,8 +1303,22 @@ private void writeObject(ObjectOutputStream oos) throws IOException {
oos.writeObject(converterProvider instanceof Serializable
? converterProvider
: null);

// [#7062] Exclude reflection cache from serialisation
for (Entry<Object, Object> 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> E[] cloneSerializables(E[] array) {
E[] clone = array.clone();

Expand All @@ -1323,6 +1343,15 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
visitListenerProviders = (VisitListenerProvider[]) ois.readObject();
transactionListenerProviders = (TransactionListenerProvider[]) ois.readObject();
converterProvider = (ConverterProvider) ois.readObject();
data = new ConcurrentHashMap<Object, Object>();

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 {
Expand Down
2 changes: 1 addition & 1 deletion jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapperProvider.java
View file
Expand Up @@ -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;

Expand Down
36 changes: 24 additions & 12 deletions jOOQ/src/main/java/org/jooq/impl/Tools.java
View file
Expand Up @@ -129,6 +129,13 @@
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;

Expand Down Expand Up @@ -469,14 +476,22 @@ enum DataKey {
* <code>new String()</code> 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
Expand Down Expand Up @@ -2684,7 +2699,7 @@ static interface CachedOperation<V> {
* @return The cached value or the outcome of the cached operation.
*/
@SuppressWarnings("unchecked")
static final <V> V run(Configuration configuration, CachedOperation<V> operation, String type, Object key) {
static final <V> V run(Configuration configuration, CachedOperation<V> operation, DataCacheKey type, Object key) {

// If no configuration is provided take the default configuration that loads the default Settings
if (configuration == null)
Expand All @@ -2696,8 +2711,6 @@ static final <V> V run(Configuration configuration, CachedOperation<V> operation

Map<Object, Object> cache = (Map<Object, Object>) configuration.data(type);
if (cache == null) {

// String synchronization is OK as all type literals were created using new String()
synchronized (type) {
cache = (Map<Object, Object>) configuration.data(type);

Expand All @@ -2709,7 +2722,6 @@ static final <V> V run(Configuration configuration, CachedOperation<V> operation
}

Object result = cache.get(key);

if (result == null) {
synchronized (cache) {
result = cache.get(key);
Expand Down

0 comments on commit 380cb70

Please sign in to comment.