JDBCDataStore initialization not thread-safe

Description

the initialization of the JDBCDataStore is done lazily:

> getClassToSqlTypeMappings() {
// (1)
classToSqlTypeMappings = >();
dialect.registerClassToSqlMappings(classToSqlTypeMappings); // (2)
}

classToSqlTypeMappings;
}

I am currently using several threads that share one JDBCDataStore and if this store is new, the threaded access causes - depending on thread timing - some unwanted behavior: While the first thread is registering several classes by its dialect (2), another thread takes the incomplete Map<Class<?>, Integer> (because it is not null (1)) and continues. For access operations like a simple Map.get(), the concurrency issue has no deep impact. But there are other lines of code e.g. in JDBCDataStore itself which will fail with an exception:

<?> clazz) {
mapping = getClassToSqlTypeMappings().get(clazz);

) {
class which matches best
List<Map.Entry< ArrayList();
// (3)
(e.getKey().isAssignableFrom(clazz) ) {
matches.add(e);
}
}

The iteration of the map in line (3) leads to a ConcurrentModificationException when another thread is registering classes for the sql mapping in this time:

Caused by: java.io.IOException
at org.geotools.jdbc.JDBCFeatureSource.getReaderInternal(JDBCFeatureSource.java:611)
at org.geotools.jdbc.JDBCFeatureStore.getReaderInternal(JDBCFeatureStore.java:225)
at org.geotools.data.store.ContentFeatureSource.getReader(ContentFeatureSource.java:562)
at org.geotools.data.store.ContentDataStore.getFeatureReader(ContentDataStore.java:417)
... 7 more
Caused by: java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
at java.util.HashMap$EntryIterator.next(HashMap.java:834)
at java.util.HashMap$EntryIterator.next(HashMap.java:832)
at org.geotools.jdbc.JDBCDataStore.getMapping(JDBCDataStore.java:611)
at org.geotools.jdbc.PreparedStatementSQLDialect.setValue(PreparedStatementSQLDialect.java:108)
at org.geotools.jdbc.JDBCDataStore.setPreparedFilterValues(JDBCDataStore.java:3176)
at org.geotools.jdbc.JDBCDataStore.setPreparedFilterValues(JDBCDataStore.java:3153)
at org.geotools.jdbc.JDBCDataStore.selectJoinSQLPS(JDBCDataStore.java:3139)
at org.geotools.jdbc.JDBCFeatureSource.getReaderInternal(JDBCFeatureSource.java:588)
... 10 more

So I modified the code in this way:

/**

  • Synchronizer access on {@link #classToSqlTypeMappings}
    */
    ();
    //...
    > getClassToSqlTypeMappings() {
    Map<> mapping = classToSqlTypeMappings;
    ) {
    (classToSqlTypeMappingsLock) {
    mapping = classToSqlTypeMappings;
    ){
    classToSqlTypeMappings = >();
    dialect.registerClassToSqlMappings(classToSqlTypeMappings);
    }
    }
    }
    classToSqlTypeMappings;
    }

The same lazy initialization is done on other members of JDBCDataStore, so I would suggest a modification for all these lines of code.

Comment by Andrea Aime:
<blockquote>Hum, that's double checked locking, in order to make it work I believe you'll also have to mark classToSqlTypeMappings as volatile</blockquote>

<hr />

Full messages from mailing list archive: http://sourceforge.net/mailarchive/message.php?msg_id=30528488 - 2nd

Environment

None

Assignee

Unassigned

Reporter

codehaus

Triage

None

Components

Fix versions

Affects versions

Priority

Medium
Configure