[GEOT-1368] Deadlock while creating a Coordinate Reference System

git-svn-id: http://svn.osgeo.org/geotools/branches/2.7.x@37023 e5c1c795-43da-0310-a71f-fac65c449510

modules/library/metadata/src/main/java/org/geotools/factory/FactoryRegistry.java

@@ -107,20 +107,19 @@ public class FactoryRegistry extends ServiceRegistry {
      * as a guard against infinite recursivity (i.e. when a factory to be scanned request
      * an other dependency of the same category).
      */
-    private final Set<Class<?>> scanningCategories = new HashSet<Class<?>>();
+    private final RecursionCheckingHelper scanningCategories = new RecursionCheckingHelper();
 
     /**
      * Factories under testing for availability. This is used by
      * {@link #isAvailable} as a guard against infinite recursivity.
      */
-    private final Set<Class<? extends OptionalFactory>> testingAvailability =
-            new HashSet<Class<? extends OptionalFactory>>();
+    private final RecursionCheckingHelper testingAvailability = new RecursionCheckingHelper();
 
     /**
      * Factories under testing for hints compatibility. This is used by
      * {@link #usesAcceptableHints} as a guard against infinite recursivity.
      */
-    private final Set<Factory> testingHints = new HashSet<Factory>();
+    private final RecursionCheckingHelper testingHints = new RecursionCheckingHelper();
 
     /**
      * Constructs a new registry for the specified category.
@@ -175,7 +174,7 @@ public FactoryRegistry(final Collection<Class<?>> categories) {
      *
      * @since 2.3
      */
-    public <T> Iterator<T> getServiceProviders(final Class<T> category, final Filter filter, final Hints hints) {
+    public synchronized <T> Iterator<T> getServiceProviders(final Class<T> category, final Filter filter, final Hints hints) {
         /*
          * The implementation of this method is very similar to the 'getUnfilteredProviders'
          * one except for filter handling. See the comments in 'getUnfilteredProviders' for
@@ -543,16 +542,14 @@ private boolean usesAcceptableHints(final Factory  factory,
          * ending loop if we don't put a 'testingHints' guard here. It is also a safety
          * against broken factory implementations.
          */
-        if (!testingHints.add(factory)) {
+        if (!testingHints.addAndCheck(factory)) {
             return false;
         }
         final Map<RenderingHints.Key, ?> implementationHints;
         try {
             implementationHints = Hints.stripNonKeys(factory.getImplementationHints());
         } finally {
-            if (!testingHints.remove(factory)) {
-                throw new AssertionError(factory); // Should never happen.
-            }
+            testingHints.removeAndCheck(factory);
         }
         if (implementationHints == null) {
             // factory was bad and did not meet contract - assume it used no Hints
@@ -661,18 +658,16 @@ private boolean isAvailable(final Object provider) {
         }
         final OptionalFactory factory = (OptionalFactory) provider;
         final Class<? extends OptionalFactory> type = factory.getClass();
-        if (!testingAvailability.add(type)) {
+        if (!testingAvailability.addAndCheck(type)) {
             throw new RecursiveSearchException(type);
         }
         try {
             return factory.isAvailable();
         } finally {
-            if (!testingAvailability.remove(type)) {
-                throw new AssertionError(type); // Should never happen.
-            }
+            testingAvailability.removeAndCheck(type);
         }
     }
-
+    
     /**
      * Returns all class loaders to be used for scanning plugins. Current implementation
      * returns the following class loaders:
@@ -760,7 +755,7 @@ public void scanForPlugins() {
      * @param category The category to scan for plug-ins.
      */
     private <T> void scanForPlugins(final Collection<ClassLoader> loaders, final Class<T> category) {
-        if (!scanningCategories.add(category)) {
+        if (!scanningCategories.addAndCheck(category)) {
             throw new RecursiveSearchException(category);
         }
         try {
@@ -790,9 +785,7 @@ private <T> void scanForPlugins(final Collection<ClassLoader> loaders, final Cla
                 log("scanForPlugins", message);
             }
         } finally {
-            if (!scanningCategories.remove(category)) {
-                throw new AssertionError(category);
-            }
+            scanningCategories.removeAndCheck(category);
         }
     }
 

modules/library/metadata/src/main/java/org/geotools/factory/RecursionCheckingHelper.java

@@ -0,0 +1,39 @@
+package org.geotools.factory;
+
+import java.util.HashSet;
+import java.util.Set;
+
+class RecursionCheckingHelper {
+
+    private final ThreadLocal<Set> threadLocalSet = new ThreadLocal<Set>();
+
+    boolean addAndCheck(Object item) {
+        Set set = threadLocalSet.get();
+        if(set == null) {
+            set = new HashSet<Class<?>>();
+            threadLocalSet.set(set);
+        }
+        return set.add(item);
+    }
+
+    boolean contains(Object item) {
+        Set<Class<?>> set = threadLocalSet.get();
+        if(set == null) {
+            return false;
+        }
+        return set.contains(item);
+    }
+
+    void removeAndCheck(Object item) {
+        Set<Class<?>> set = threadLocalSet.get();
+        if(set == null) {
+            throw new AssertionError(null); // Should never happen.
+        } else if(!set.remove(item)) {
+            throw new AssertionError(item); // Should never happen.
+        } 
+        if(set.isEmpty()) {
+            threadLocalSet.remove();
+        }
+    }
+
+}

modules/library/referencing/src/main/java/org/geotools/referencing/factory/AllAuthoritiesFactory.java

@@ -89,7 +89,7 @@ public Set<String> getAuthorityNames() {
      * recreate the set.
      */
     @Override
-    synchronized Collection<AuthorityFactory> getFactories() {
+    Collection<AuthorityFactory> getFactories() {
         final Collection<String> authorities = ReferencingFactoryFinder.getAuthorityNames();
         if (authorities != authorityNames) {
             authorityNames = authorities;

modules/library/referencing/src/main/java/org/geotools/referencing/factory/BufferedAuthorityFactory.java

@@ -88,7 +88,7 @@ public class BufferedAuthorityFactory extends AbstractAuthorityFactory implement
      * @see #getBackingStore
      * @see DeferredAuthorityFactory#createBackingStore
      */
-    AbstractAuthorityFactory backingStore;
+    volatile AbstractAuthorityFactory backingStore;
 
     /**
      * The pool of cached objects.
@@ -228,7 +228,7 @@ AbstractAuthorityFactory getBackingStore() throws FactoryException {
      * {@link DeferredAuthorityFactory#createBackingStore} throws an exception.
      */
     @Override
-    synchronized boolean isAvailable() {
+    boolean isAvailable() {
         try {
             return getBackingStore().isAvailable();
         } catch (FactoryNotFoundException exception) {
@@ -289,15 +289,15 @@ boolean sameAuthorityCodes(final AuthorityFactory factory) {
      * Returns the vendor responsible for creating the underlying factory implementation.
      */
     @Override
-    public synchronized Citation getVendor() {
+    public Citation getVendor() {
         return (backingStore!=null) ? backingStore.getVendor() : super.getVendor();
     }
 
     /**
      * Returns the organization or party responsible for definition and maintenance of the
      * underlying database.
      */
-    public synchronized Citation getAuthority() {
+    public Citation getAuthority() {
         return (backingStore!=null) ? backingStore.getAuthority() : null;
     }
 
@@ -308,7 +308,7 @@ public synchronized Citation getAuthority() {
      * @throws FactoryException if a failure occured while fetching the engine description.
      */
     @Override
-    public synchronized String getBackingStoreDescription() throws FactoryException {
+    public String getBackingStoreDescription() throws FactoryException {
         return getBackingStore().getBackingStoreDescription();
     }
 
@@ -322,7 +322,7 @@ public synchronized String getBackingStoreDescription() throws FactoryException
      *         returns an {@linkplain java.util.Collections#EMPTY_SET empty set}.
      * @throws FactoryException if access to the underlying database failed.
      */
-    public synchronized Set<String> getAuthorityCodes(final Class<? extends IdentifiedObject> type)
+    public Set<String> getAuthorityCodes(final Class<? extends IdentifiedObject> type)
             throws FactoryException
     {
         return getBackingStore().getAuthorityCodes(type);
@@ -337,7 +337,7 @@ public synchronized Set<String> getAuthorityCodes(final Class<? extends Identifi
      * @throws NoSuchAuthorityCodeException if the specified {@code code} was not found.
      * @throws FactoryException if the query failed for some other reason.
      */
-    public synchronized InternationalString getDescriptionText(final String code)
+    public InternationalString getDescriptionText(final String code)
             throws NoSuchAuthorityCodeException, FactoryException
     {
         return getBackingStore().getDescriptionText(code);

modules/library/referencing/src/main/java/org/geotools/referencing/factory/DeferredAuthorityFactory.java

@@ -125,13 +125,16 @@ public boolean isAvailable() {
      */
     @Override
     final AbstractAuthorityFactory getBackingStore() throws FactoryException {
-        assert Thread.holdsLock(this);
         if (backingStore == null) {
-            backingStore = createBackingStore();
-            if (backingStore == null) {
-                throw new FactoryNotFoundException(Errors.format(ErrorKeys.NO_DATA_SOURCE));
+            synchronized (this) {
+                if(backingStore == null) {
+                    backingStore = createBackingStore();
+                    if (backingStore == null) {
+                        throw new FactoryNotFoundException(Errors.format(ErrorKeys.NO_DATA_SOURCE));
+                    }
+                    completeHints();
+                }
             }
-            completeHints();
         }
         used = true; // Tell to the disposer to wait again.
         return backingStore;

modules/library/referencing/src/main/java/org/geotools/referencing/factory/ManyAuthoritiesFactory.java

@@ -136,8 +136,7 @@ Collection<AuthorityFactory> getFactories() {
      * Sets the factories. This method is invoked by the {@link AllAuthoritiesFactory} subclass
      * only. No one else should invoke this method, since factories should be immutable.
      */
-    final void setFactories(final Collection<AuthorityFactory> factories) {
-        assert Thread.holdsLock(this);
+    synchronized final void setFactories(final Collection<AuthorityFactory> factories) {
         this.factories = createFallbacks(factories);
     }
 

modules/library/referencing/src/main/java/org/geotools/referencing/factory/ThreadedAuthorityFactory.java

@@ -217,7 +217,7 @@ AbstractAuthorityFactory getBackingStore() throws FactoryException {
      * {@link DeferredAuthorityFactory#createBackingStore} throws an exception.
      */
     @Override
-    synchronized boolean isAvailable() {
+    boolean isAvailable() {
         try {
             return getBackingStore().isAvailable();
         } catch (FactoryNotFoundException exception) {
@@ -279,15 +279,15 @@ boolean sameAuthorityCodes(final AuthorityFactory factory) {
      * Returns the vendor responsible for creating the underlying factory implementation.
      */
     @Override
-    public synchronized Citation getVendor() {
+    public Citation getVendor() {
         return (backingStore!=null) ? backingStore.getVendor() : super.getVendor();
     }
 
     /**
      * Returns the organization or party responsible for definition and maintenance of the
      * underlying database.
      */
-    public synchronized Citation getAuthority() {
+    public Citation getAuthority() {
         return (backingStore!=null) ? backingStore.getAuthority() : null;
     }
 
@@ -298,7 +298,7 @@ public synchronized Citation getAuthority() {
      * @throws FactoryException if a failure occured while fetching the engine description.
      */
     @Override
-    public synchronized String getBackingStoreDescription() throws FactoryException {
+    public String getBackingStoreDescription() throws FactoryException {
         return getBackingStore().getBackingStoreDescription();
     }
 
@@ -312,7 +312,7 @@ public synchronized String getBackingStoreDescription() throws FactoryException
      *         returns an {@linkplain java.util.Collections#EMPTY_SET empty set}.
      * @throws FactoryException if access to the underlying database failed.
      */
-    public synchronized Set<String> getAuthorityCodes(final Class type)
+    public Set<String> getAuthorityCodes(final Class type)
             throws FactoryException
     {
         return getBackingStore().getAuthorityCodes(type);
@@ -327,7 +327,7 @@ public synchronized Set<String> getAuthorityCodes(final Class type)
      * @throws NoSuchAuthorityCodeException if the specified {@code code} was not found.
      * @throws FactoryException if the query failed for some other reason.
      */
-    public synchronized InternationalString getDescriptionText(final String code)
+    public InternationalString getDescriptionText(final String code)
             throws FactoryException
     {
         return getBackingStore().getDescriptionText(code);

modules/library/referencing/src/main/java/org/geotools/referencing/factory/epsg/DirectEpsgFactory.java

@@ -313,7 +313,7 @@ private static void setBursaWolfParameter(final BursaWolfParameters parameters,
      * This authority will contains the database version in the {@linkplain Citation#getEdition
      * edition} attribute, together with the {@linkplain Citation#getEditionDate edition date}.
      */
-    private transient Citation authority;
+    private volatile transient Citation authority;
 
     /**
      * Last object type returned by {@link #createObject}, or -1 if none.
@@ -466,30 +466,34 @@ public DirectEpsgFactory(final Hints userHints, final DataSource dataSource) {
      * This authority will contains the database version in the {@linkplain Citation#getEdition
      * edition} attribute, together with the {@linkplain Citation#getEditionDate edition date}.
      */
-    public synchronized Citation getAuthority() {
+    public Citation getAuthority() {
         if (authority == null) try {
-            // we sort on version_number too since in v7.4 they had two entries with the same version date
-            final String query = adaptSQL("SELECT VERSION_NUMBER, VERSION_DATE FROM [Version History]" +
-                                          " ORDER BY VERSION_DATE DESC, VERSION_NUMBER DESC");
-            final DatabaseMetaData metadata  = getConnection().getMetaData();
-            final Statement        statement = getConnection().createStatement();
-            final ResultSet        result    = statement.executeQuery(query);
-            if (result.next()) {
-                final String version = result.getString(1);
-                final Date   date    = result.getDate  (2);
-                final String engine  = metadata.getDatabaseProductName();
-                final CitationImpl c = new CitationImpl(Citations.EPSG);
-                c.getAlternateTitles().add(Vocabulary.formatInternational(
-                        VocabularyKeys.DATA_BASE_$3, "EPSG", version, engine));
-                c.setEdition(new SimpleInternationalString(version));
-                c.setEditionDate(date);
-                authority = (Citation) c.unmodifiable();
-                hints.put(Hints.VERSION, new Version(version));  // For getImplementationHints()
-            } else {
-                authority = Citations.EPSG;
+            synchronized (this) {
+                if(authority == null) {
+                    // we sort on version_number too since in v7.4 they had two entries with the same version date
+                    final String query = adaptSQL("SELECT VERSION_NUMBER, VERSION_DATE FROM [Version History]" +
+                                                  " ORDER BY VERSION_DATE DESC, VERSION_NUMBER DESC");
+                    final DatabaseMetaData metadata  = getConnection().getMetaData();
+                    final Statement        statement = getConnection().createStatement();
+                    final ResultSet        result    = statement.executeQuery(query);
+                    if (result.next()) {
+                        final String version = result.getString(1);
+                        final Date   date    = result.getDate  (2);
+                        final String engine  = metadata.getDatabaseProductName();
+                        final CitationImpl c = new CitationImpl(Citations.EPSG);
+                        c.getAlternateTitles().add(Vocabulary.formatInternational(
+                                VocabularyKeys.DATA_BASE_$3, "EPSG", version, engine));
+                        c.setEdition(new SimpleInternationalString(version));
+                        c.setEditionDate(date);
+                        authority = (Citation) c.unmodifiable();
+                        hints.put(Hints.VERSION, new Version(version));  // For getImplementationHints()
+                    } else {
+                        authority = Citations.EPSG;
+                    }
+                    result.close();
+                    statement.close();
+                }
             }
-            result.close();
-            statement.close();
         } catch (SQLException exception) {
             Logging.unexpectedException(LOGGER, DirectEpsgFactory.class, "getAuthority", exception);
             return Citations.EPSG;