[Geotools-devel] [jira] (GEOT-4506) JDBCDataStore initialization not thread-safe

Wed, 10 Jul 2013 01:00:19 -0700

Issue Type: Bug Bug
Affects Versions: 8.0
Assignee: Unassigned
Components: jdbc
Created: 10/Jul/13 2:57 AM
Description:

the initialization of the JDBCDataStore is done lazily:

    public Map<Class<?>, Integer> getClassToSqlTypeMappings() {
        if (classToSqlTypeMappings == null) { // (1)
            classToSqlTypeMappings = new HashMap<Class<?>, Integer>();
            dialect.registerClassToSqlMappings(classToSqlTypeMappings); // (2)
        }

        return 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:

    public Integer getMapping(Class<?> clazz) {
        Integer mapping = getClassToSqlTypeMappings().get(clazz);

        if (mapping == null) {
            //no match, try a "fuzzy" match in which we find the super class which matches best
            List<Map.Entry<Class<?>, Integer>> matches = new ArrayList();
            for (Map.Entry<Class<?>, Integer> e : getClassToSqlTypeMappings().entrySet()) { // (3)
                if (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 for access on {@link #classToSqlTypeMappings}
     */
    private final Object classToSqlTypeMappingsLock = new Object();
    //...
    public Map<Class<?>, Integer> getClassToSqlTypeMappings() {
       Map<Class<?>, Integer> mapping = classToSqlTypeMappings;
        if (mapping == null) {
             synchronized (classToSqlTypeMappingsLock) {
                    mapping = classToSqlTypeMappings;
                    if(mapping == null){
                           classToSqlTypeMappings = new HashMap<Class<?>, Integer>();
                           dialect.registerClassToSqlMappings(classToSqlTypeMappings);
                    }
             }
        }
        return 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:

Hum, that's double checked locking, in order to make it work I believe you'll also have to mark classToSqlTypeMappings as volatile

Full messages from mailing list archive: 1st, 2nd
Environment: Geotools 8.0
Java 1.6
Project: GeoTools
Priority: Major Major
Reporter: Robin Pradel
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira 
------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to