svn commit: r1624425 - /sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java

[de...@apache.org]

Author: desruisseaux
Date: Fri Sep 12 00:05:00 2014
New Revision: 1624425

URL: http://svn.apache.org/r1624425
Log:
Synchronization problem.

Modified:
    sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java

Modified: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java?rev=1624425&r1=1624424&r2=1624425&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/util/iso/DefaultNameFactory.java [UTF-8] Fri Sep 12 00:05:00 2014
@@ -427,10 +427,27 @@ public class DefaultNameFactory extends 
         if (valueClass == null) {
             return null;
         }
+        /*
+         * Note: we do not cache the TypeName for the valueClass argument because:
+         * - It is not needed (at least in the default implementation) for getting unique instance.
+         * - It is not the best place for performance improvement, since TypeName are usually only
+         *   a step in the creation of bigger object (typically AttributeType). Users are better to
+         *   cache the bigger object instead.
+         */
         TypeNames t = typeNames;
-        if (t == null) synchronized (this) {
-            // Double-check strategy is ok if 'typeNames' is volatile.
-            typeNames = t = new TypeNames(this);
+        if (t == null) {
+            synchronized (pool) {
+                /*
+                 * Double-check strategy is ok if 'typeNames' is volatile. We synchronize on 'pool' because
+                 * the TypeNames constructor will call back methods from this class, which will use the pool.
+                 * We are better to use only one lock for reducing the risk of dead-lock. Inconvenient is that
+                 * we potentially invoke user code (since our methods are overrideable) while holding the lock.
+                 */
+                t = typeNames;
+                if (t == null) {
+                    typeNames = t = new TypeNames(this);
+                }
+            }
         }
         return t.toTypeName(this, valueClass);
     }