[dna-commits] DNA SVN: r897 - trunk/dna-jcr/src/main/java/org/jboss/dna/jcr.

[dna-commits at lists.jboss.org]

Author: rhauch
Date: 2009-05-07 10:43:54 -0400 (Thu, 07 May 2009)
New Revision: 897

Modified:
   trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
Log:
DNA-398 RepositoryNodeTypeManager is Not Thread-Safe

Corrected the public getAllNodeTypes() method, which was returning the actually values collection associated with an internal map that is modified.  This means that although the method atomically returns the collection in a thread-safe manner, the resulting collection is live and may be modified via other calls.  The change is to create a defensive and immutable copy of the node types within the lock scope.  Currently, this method is only called from the JcrNodeTypeManager implementation (the Session-specific wrapper around the shared RepositoryNodeTypeManager), which is not currently called from within any DNA JCR implementation code.  Therefore, the cost of making a copy has little impact.

Also added were JavaDoc for several public methods.

Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java	2009-05-07 14:30:53 UTC (rev 896)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java	2009-05-07 14:43:54 UTC (rev 897)
@@ -104,11 +104,11 @@
 
     private final ExecutionContext context;
 
-    @GuardedBy("nodeTypeManagerLock")
+    @GuardedBy( "nodeTypeManagerLock" )
     private final Map<Name, JcrNodeType> nodeTypes;
-    @GuardedBy("nodeTypeManagerLock")
+    @GuardedBy( "nodeTypeManagerLock" )
     private final Map<PropertyDefinitionId, JcrPropertyDefinition> propertyDefinitions;
-    @GuardedBy("nodeTypeManagerLock")
+    @GuardedBy( "nodeTypeManagerLock" )
     private final Map<NodeDefinitionId, JcrNodeDefinition> childNodeDefinitions;
     private final PropertyFactory propertyFactory;
     private final PathFactory pathFactory;
@@ -146,19 +146,30 @@
         nodeTypes = new HashMap<Name, JcrNodeType>(50);
     }
 
+    /**
+     * Return an immutable snapshot of the node types that are currently registered in this node type manager.
+     * 
+     * @return the immutable collection of (immutable) node types; never null
+     */
     public Collection<JcrNodeType> getAllNodeTypes() {
         try {
             nodeTypeManagerLock.readLock().lock();
-            return nodeTypes.values();
+            return Collections.unmodifiableCollection(new ArrayList<JcrNodeType>(nodeTypes.values()));
         } finally {
             nodeTypeManagerLock.readLock().unlock();
         }
     }
 
+    /**
+     * Return an immutable snapshot of the mixin node types that are currently registered in this node type manager.
+     * 
+     * @return the immutable collection of (immutable) mixin node types; never null
+     * @see #getPrimaryNodeTypes()
+     */
     public Collection<JcrNodeType> getMixinNodeTypes() {
         try {
             nodeTypeManagerLock.readLock().lock();
-            
+
             List<JcrNodeType> types = new ArrayList<JcrNodeType>(nodeTypes.size());
 
             for (JcrNodeType nodeType : nodeTypes.values()) {
@@ -171,6 +182,12 @@
         }
     }
 
+    /**
+     * Return an immutable snapshot of the primary node types that are currently registered in this node type manager.
+     * 
+     * @return the immutable collection of (immutable) primary node types; never null
+     * @see #getMixinNodeTypes()
+     */
     public Collection<JcrNodeType> getPrimaryNodeTypes() {
         try {
             nodeTypeManagerLock.readLock().lock();