aries-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gno...@apache.org
Subject svn commit: r1343938 - /aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java
Date Tue, 29 May 2012 19:24:51 GMT
Author: gnodet
Date: Tue May 29 19:24:50 2012
New Revision: 1343938

URL: http://svn.apache.org/viewvc?rev=1343938&view=rev
Log:
[ARIES-858] Fix concurrency issue with NamespaceHandlerRegistryImpl causing application test
failures


Conflicts:

	blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java

Modified:
    aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java

Modified: aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java
URL: http://svn.apache.org/viewvc/aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java?rev=1343938&r1=1343937&r2=1343938&view=diff
==============================================================================
--- aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java
(original)
+++ aries/branches/blueprint-0.3.2-fixes/blueprint-core/src/main/java/org/apache/aries/blueprint/namespace/NamespaceHandlerRegistryImpl.java
Tue May 29 19:24:50 2012
@@ -40,8 +40,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CopyOnWriteArraySet;
 import javax.xml.XMLConstants;
 import javax.xml.transform.Source;
 import javax.xml.transform.stream.StreamSource;
@@ -77,26 +78,38 @@ public class NamespaceHandlerRegistryImp
 
     private static final Logger LOGGER = LoggerFactory.getLogger(NamespaceHandlerRegistryImpl.class);
 
+    // The bundle context is thread safe
     private final BundleContext bundleContext;
-    private final Map<URI, Set<NamespaceHandler>> handlers;
+
+    // The service tracker is thread safe
     private final ServiceTracker tracker;
-    private final Map<Map<URI, NamespaceHandler>, Reference<Schema>> schemas
= new LRUMap<Map<URI, NamespaceHandler>, Reference<Schema>>(10);
-    private SchemaFactory schemaFactory;
-    private List<NamespaceHandlerSetImpl> sets;
-    private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+    // The handlers map is concurrent
+    private final ConcurrentHashMap<URI, CopyOnWriteArraySet<NamespaceHandler>>
handlers;
+
+    // Access to the LRU schemas map is synchronized on itself
+    private final Map<Map<URI, NamespaceHandler>, Reference<Schema>> schemas
=
+                        new LRUMap<Map<URI, NamespaceHandler>, Reference<Schema>>(10);
+
+    // Access to this factory is synchronized on itself
+    private final SchemaFactory schemaFactory;
+
+    // Access to this variable is not synchronized.  The list itself is concurrent
+    private final CopyOnWriteArrayList<NamespaceHandlerSetImpl> sets;
 
     public NamespaceHandlerRegistryImpl(BundleContext bundleContext) {
         this.bundleContext = bundleContext;
-        handlers = new HashMap<URI, Set<NamespaceHandler>>();
-        sets = new ArrayList<NamespaceHandlerSetImpl>();
+        handlers = new ConcurrentHashMap<URI, CopyOnWriteArraySet<NamespaceHandler>>();
+        sets = new CopyOnWriteArrayList<NamespaceHandlerSetImpl>();
+        schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
         tracker = new ServiceTracker(bundleContext, NamespaceHandler.class.getName(), this);
         tracker.open();
     }
 
     public Object addingService(ServiceReference reference) {
-        LOGGER.debug("Adding NamespaceHandler "+reference.toString());
+        LOGGER.debug("Adding NamespaceHandler " + reference.toString());
         NamespaceHandler handler = (NamespaceHandler) bundleContext.getService(reference);
-        if(handler!=null){
+        if (handler != null) {
             try {
                 Map<String, Object> props = new HashMap<String, Object>();
                 for (String name : reference.getPropertyKeys()) {
@@ -130,13 +143,12 @@ public class NamespaceHandlerRegistryImp
         }
     }
 
-    public synchronized void registerHandler(NamespaceHandler handler, Map properties) {
+    public void registerHandler(NamespaceHandler handler, Map properties) {
         List<URI> namespaces = getNamespaces(properties);
         for (URI uri : namespaces) {
-            Set<NamespaceHandler> h = handlers.get(uri);
+            CopyOnWriteArraySet<NamespaceHandler> h = handlers.putIfAbsent(uri, new
CopyOnWriteArraySet<NamespaceHandler>());
             if (h == null) {
-                h = new HashSet<NamespaceHandler>();
-                handlers.put(uri, h);
+                h = handlers.get(uri);
             }
             if (h.add(handler)) {
                 for (NamespaceHandlerSetImpl s : sets) {
@@ -146,18 +158,28 @@ public class NamespaceHandlerRegistryImp
         }
     }
 
-    public synchronized void unregisterHandler(NamespaceHandler handler, Map properties)
{
+    public void unregisterHandler(NamespaceHandler handler, Map properties) {
         List<URI> namespaces = getNamespaces(properties);
         for (URI uri : namespaces) {
-            Set<NamespaceHandler> h = handlers.get(uri);
-            if (h == null || !h.remove(handler)) {
+            CopyOnWriteArraySet<NamespaceHandler> h = handlers.get(uri);
+            if (!h.remove(handler)) {
                 continue;
             }
             for (NamespaceHandlerSetImpl s : sets) {
                 s.unregisterHandler(uri, handler);
             }
         }
-        removeSchemasFor(handler);
+        synchronized (schemas) {
+            List<Map<URI, NamespaceHandler>> keys = new ArrayList<Map<URI,
NamespaceHandler>>();
+            for (Map<URI, NamespaceHandler> key : schemas.keySet()) {
+                if (key.values().contains(handler)) {
+                    keys.add(key);
+                }
+            }
+            for (Map<URI, NamespaceHandler> key : keys) {
+                schemas.remove(key);
+            }
+        }
     }
 
     private static List<URI> getNamespaces(Map properties) {
@@ -206,7 +228,7 @@ public class NamespaceHandlerRegistryImp
         }
     }
 
-    public synchronized NamespaceHandlerSet getNamespaceHandlers(Set<URI> uris, Bundle
bundle) {
+    public NamespaceHandlerSet getNamespaceHandlers(Set<URI> uris, Bundle bundle) {
         NamespaceHandlerSetImpl s = new NamespaceHandlerSetImpl(uris, bundle);
         sets.add(s);
         return s;
@@ -227,26 +249,19 @@ public class NamespaceHandlerRegistryImp
         // If it contains additional namespaces, it should not be a problem since
         // they won't be used at all
         if (schemaMap == null || schemaMap.isEmpty()) {
-            try {
-                lock.readLock().lock();
-                for (Map<URI, NamespaceHandler> key : schemas.keySet()) {
-                    boolean found = true;
-                    for (URI uri : handlers.keySet()) {
-                        if (!handlers.get(uri).equals(key.get(uri))) {
-                            found = false;
-                            break;
-                        }
-                    }
-                    if (found) {
-                        return schemas.get(key).get();
-                    }
-                }
-            } finally {
-                lock.readLock().unlock();
+            Schema schema = getExistingSchema(handlers);
+            if (schema != null) {
+                return schema;
             }
         }
-        try {
-            lock.writeLock().lock();
+        synchronized (schemaFactory) {
+            // Just double check in case the schema has just been created
+            if (schemaMap == null || schemaMap.isEmpty()) {
+                Schema schema = getExistingSchema(handlers);
+                if (schema != null) {
+                    return schema;
+                }
+            }
             final List<StreamSource> schemaSources = new ArrayList<StreamSource>();
             try {
                 schemaSources.add(new StreamSource(getClass().getResourceAsStream("/org/apache/aries/blueprint/blueprint.xsd")));
@@ -268,8 +283,7 @@ public class NamespaceHandlerRegistryImp
                         schemaSources.add(new StreamSource(url.openStream(), url.toExternalForm()));
                     }
                 }
-                SchemaFactory factory = getSchemaFactory();
-                factory.setResourceResolver(new LSResourceResolver() {
+                schemaFactory.setResourceResolver(new LSResourceResolver() {
                     public LSInput resolveResource(String type,
                                                    final String namespaceURI,
                                                    final String publicId,
@@ -329,28 +343,30 @@ public class NamespaceHandlerRegistryImp
                     }
 
                 });
-                Schema schema = factory.newSchema(schemaSources.toArray(new Source[schemaSources.size()]));
-                // Remove schemas that are fully included
-                for (Iterator<Map<URI, NamespaceHandler>> iterator = schemas.keySet().iterator();
iterator.hasNext();) {
-                    Map<URI, NamespaceHandler> key = iterator.next();
-                    boolean found = true;
-                    for (URI uri : key.keySet()) {
-                        if (!key.get(uri).equals(handlers.get(uri))) {
-                            found = false;
+                Schema schema = schemaFactory.newSchema(schemaSources.toArray(new Source[schemaSources.size()]));
+                synchronized (schemas) {
+                    // Remove schemas that are fully included
+                    for (Iterator<Map<URI, NamespaceHandler>> iterator = schemas.keySet().iterator();
iterator.hasNext();) {
+                        Map<URI, NamespaceHandler> key = iterator.next();
+                        boolean found = true;
+                        for (URI uri : key.keySet()) {
+                            if (!key.get(uri).equals(handlers.get(uri))) {
+                                found = false;
+                                break;
+                            }
+                        }
+                        if (found) {
+                            iterator.remove();
                             break;
                         }
                     }
-                    if (found) {
-                        iterator.remove();
-                        break;
+                    // Add our new schema
+                    if (schemaMap.isEmpty()) {
+                        //only cache non-custom schemas
+                        schemas.put(handlers, new SoftReference<Schema>(schema));
                     }
+                    return schema;
                 }
-                // Add our new schema
-                if (schemaMap.isEmpty()) {
-                    //only cache non-custom schemas
-                    schemas.put(handlers, new SoftReference<Schema>(schema));
-                }
-                return schema;
             } finally {
                 for (StreamSource s : schemaSources) {
                     try {
@@ -360,8 +376,24 @@ public class NamespaceHandlerRegistryImp
                     }
                 }
             }
-        } finally {
-            lock.writeLock().unlock();
+        }
+    }
+
+    private Schema getExistingSchema(Map<URI, NamespaceHandler> handlers) {
+        synchronized (schemas) {
+            for (Map<URI, NamespaceHandler> key : schemas.keySet()) {
+                boolean found = true;
+                for (URI uri : handlers.keySet()) {
+                    if (!handlers.get(uri).equals(key.get(uri))) {
+                        found = false;
+                        break;
+                    }
+                }
+                if (found) {
+                    return schemas.get(key).get();
+                }
+            }
+            return null;
         }
     }
 
@@ -416,25 +448,6 @@ public class NamespaceHandlerRegistryImp
         }
     };
 
-    protected synchronized void removeSchemasFor(NamespaceHandler handler) {
-        List<Map<URI, NamespaceHandler>> keys = new ArrayList<Map<URI,
NamespaceHandler>>();
-        for (Map<URI, NamespaceHandler> key : schemas.keySet()) {
-            if (key.values().contains(handler)) {
-                keys.add(key);
-            }
-        }
-        for (Map<URI, NamespaceHandler> key : keys) {
-            schemas.remove(key);
-        }
-    }
-
-    private SchemaFactory getSchemaFactory() {
-        if (schemaFactory == null) {
-            schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
-        }
-        return schemaFactory;
-    }
-
     protected class NamespaceHandlerSetImpl implements NamespaceHandlerSet {
 
         private final Map<Listener, Boolean> listeners;
@@ -448,7 +461,7 @@ public class NamespaceHandlerRegistryImp
             this.listeners = new HashMap<Listener, Boolean>();
             this.namespaces = namespaces;
             this.bundle = bundle;
-            handlers = new HashMap<URI, NamespaceHandler>();
+            this.handlers = new HashMap<URI, NamespaceHandler>();
             for (URI ns : namespaces) {
                 findCompatibleNamespaceHandler(ns);
             }
@@ -502,11 +515,11 @@ public class NamespaceHandlerRegistryImp
             return schema;
         }
 
-        public synchronized void addListener(Listener listener) {
+        public void addListener(Listener listener) {
             listeners.put(listener, Boolean.TRUE);
         }
 
-        public synchronized void removeListener(Listener listener) {
+        public void removeListener(Listener listener) {
             listeners.remove(listener);
         }
 



Mime
View raw message