felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From uiter...@apache.org
Subject svn commit: r1488970 - in /felix: sandbox/uiterlix/dependencymanager/core/ trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ trunk/dependencymanager/core/sr...
Date Mon, 03 Jun 2013 12:13:10 GMT
Author: uiterlix
Date: Mon Jun  3 12:13:09 2013
New Revision: 1488970

URL: http://svn.apache.org/r1488970
Log:
Resolved FELIX-4014 (possible deadlock on ServiceDependencyImpl.handleAspectAwareRemoved), FELIX-4097 (debug logging in ServiceDependency), FELIX-4098 (wrong swap order with multiple threads), FELIX-4099 (support for negate in multi property filter index).

Added:
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Filter.java
      - copied, changed from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Filter.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/MultiPropertyFilterIndex.java
      - copied, changed from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/AdvancedMultiPropertyFilterIndex.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Property.java
      - copied, changed from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Property.java
Removed:
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/MultiPropertyExactFilter.java
Modified:
    felix/sandbox/uiterlix/dependencymanager/core/pom.xml
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/DependencyManager.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ServiceDependency.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ResourceDependencyImpl.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ServiceDependencyImpl.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/BundleContextInterceptor.java
    felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/ServiceRegistryCache.java

Modified: felix/sandbox/uiterlix/dependencymanager/core/pom.xml
URL: http://svn.apache.org/viewvc/felix/sandbox/uiterlix/dependencymanager/core/pom.xml?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/sandbox/uiterlix/dependencymanager/core/pom.xml (original)
+++ felix/sandbox/uiterlix/dependencymanager/core/pom.xml Mon Jun  3 12:13:09 2013
@@ -85,7 +85,7 @@
                         <Bundle-Name>Apache Felix Dependency Manager</Bundle-Name>
                         <Bundle-Description>Provides dynamic service and component dependency management.</Bundle-Description>
                         <Bundle-Vendor>The Apache Software Foundation</Bundle-Vendor>
-                        <Export-Package>org.apache.felix.dm;version="3.0.0";provide:=true,org.apache.felix.dm.tracker;version="3.1.0";provide:=true</Export-Package>
+                        <Export-Package>org.apache.felix.dm;version="3.1.1";provide:=true,org.apache.felix.dm.tracker;version="3.1.0";provide:=true</Export-Package>
                         <Import-Package>*</Import-Package>
                         <Private-Package>org.apache.felix.dm.impl, org.apache.felix.dm.impl.*</Private-Package>
                         <Include-Resource>src/main/java</Include-Resource>

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/DependencyManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/DependencyManager.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/DependencyManager.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/DependencyManager.java Mon Jun  3 12:13:09 2013
@@ -39,9 +39,9 @@ import org.apache.felix.dm.impl.dependen
 import org.apache.felix.dm.impl.dependencies.ServiceDependencyImpl;
 import org.apache.felix.dm.impl.dependencies.TemporalServiceDependencyImpl;
 import org.apache.felix.dm.impl.index.AspectFilterIndex;
-import org.apache.felix.dm.impl.index.MultiPropertyExactFilter;
 import org.apache.felix.dm.impl.index.AdapterFilterIndex;
 import org.apache.felix.dm.impl.index.ServiceRegistryCache;
+import org.apache.felix.dm.impl.index.multiproperty.MultiPropertyFilterIndex;
 import org.apache.felix.dm.impl.metatype.PropertyMetaDataImpl;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
@@ -110,8 +110,7 @@ public class DependencyManager {
                     	m_serviceRegistryCache.addFilterIndex(new AdapterFilterIndex());
                     }
                     else {
-                        String[] propList = props[i].split(",");
-                        m_serviceRegistryCache.addFilterIndex(new MultiPropertyExactFilter(propList));
+                    	m_serviceRegistryCache.addFilterIndex(new MultiPropertyFilterIndex(props[i]));
                     }
                 }
             }
@@ -146,7 +145,6 @@ public class DependencyManager {
     
     private BundleContext createContext(BundleContext context) {
         if (m_serviceRegistryCache != null) {
-//            System.out.println("DM: Enabling bundle context interceptor for bundle #" + context.getBundle().getBundleId());
             return m_serviceRegistryCache.createBundleContextInterceptor(context);
         }
         else {

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ServiceDependency.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ServiceDependency.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ServiceDependency.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/ServiceDependency.java Mon Jun  3 12:13:09 2013
@@ -214,4 +214,11 @@ public interface ServiceDependency exten
 	 * @param isInstanceBound <code>true</code> if this dependency should be instance bound
 	 */
     public ServiceDependency setInstanceBound(boolean isInstanceBound);    
+    
+    /**
+     * Enabled debug logging for this dependency instance. The logging is prefixed with the given identifier.
+     * @param identifier
+     * @return this service dependency.
+     */
+    public ServiceDependency setDebug(String identifier);
 }

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ResourceDependencyImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ResourceDependencyImpl.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ResourceDependencyImpl.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ResourceDependencyImpl.java Mon Jun  3 12:13:09 2013
@@ -200,6 +200,10 @@ public class ResourceDependencyImpl exte
     		long counter;
     		Object[] services;
     		synchronized (this) {
+    			if (m_resources.indexOf(resource) == -1) {
+    				m_logger.log(Logger.LOG_WARNING, "handleResourceRemoved called for unknown resource: " + resource);
+    				return;
+    			}
     			m_resourceProperties.remove(m_resources.indexOf(resource));
     		    m_resources.remove(resource);
     			counter = m_resources.size();

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ServiceDependencyImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ServiceDependencyImpl.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ServiceDependencyImpl.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/dependencies/ServiceDependencyImpl.java Mon Jun  3 12:13:09 2013
@@ -28,6 +28,7 @@ import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -83,6 +84,16 @@ public class ServiceDependencyImpl exten
     private String m_propagateCallbackMethod;
     private final Map m_sr = new HashMap(); /* <DependencyService, Set<Tuple<ServiceReference, Object>> */
 	private Map m_componentByRank = new HashMap(); /* <Component, Map<Long, Map<Integer, Tuple>>> */
+	private boolean m_debug = false;
+	private String m_debugKey = null;
+	
+	private final LinkedList m_injectionQueue = new LinkedList();
+	
+	public ServiceDependency setDebug(String identifier) {
+		this.m_debug = true;
+		this.m_debugKey = identifier;
+		return this;
+	}
     
     private static final Comparator COMPARATOR = new Comparator() {
         public int getRank(ServiceReference ref) {
@@ -416,7 +427,13 @@ public class ServiceDependencyImpl exten
             }
         }
         if (needsStarting) {
-            m_tracker.open();
+        	// when the swapped callback is set, also track the aspects
+        	boolean trackAllServices = false;
+        	boolean trackAllAspects = false;
+        	if (m_callbackSwapped != null) {
+        		trackAllAspects = true;
+        	} 
+        	m_tracker.open(trackAllServices, trackAllAspects);
         }
     }
 
@@ -448,6 +465,9 @@ public class ServiceDependencyImpl exten
     }
 
     public void addedService(ServiceReference ref, Object service) {
+    	if (m_debug) {
+    		log("addedservice: " + ref);
+    	}
         boolean makeAvailable = makeAvailable();
         
         Object[] services;
@@ -458,20 +478,32 @@ public class ServiceDependencyImpl exten
             DependencyService ds = (DependencyService) services[i];
             if (makeAvailable) {
                 if (ds.isInstantiated() && isInstanceBound() && isRequired()) {
-                    invokeAdded(ds, ref, service);
+                	if (m_debug) {
+                		log("invoke added: " + ref);
+                	}
+                    invokeAdded(ds, ref, service); //**
+                }
+                // The dependency callback will be deferred until all required dependency are available.
+                if (m_debug) {
+                	log("dependency available: " + ref);
                 }
-                // The dependency callback will be defered until all required dependency are available.
-                ds.dependencyAvailable(this);
+                ds.dependencyAvailable(this); //**
                 if (!isRequired()) {
                     // For optional dependency, we always invoke callback, because at this point, we know
                     // that the service has been started, and the service start method has been called.
                     // (See the ServiceImpl.bindService method, which will activate optional dependencies using 
-                    // startTrackingOptional() method). 
-                    invokeAdded(ds, ref, service);
+                    // startTrackingOptional() method).
+                	if (m_debug) {
+                		log("invoke added: " + ref);
+                	}
+                    invokeAdded(ds, ref, service); //**
                 }
             }
             else {
-                ds.dependencyChanged(this);
+            	if (m_debug) {
+            		log("dependency changed: " + ref);
+            	}
+                ds.dependencyChanged(this); //**
                 // At this point, either the dependency is optional (meaning that the service has been started,
                 // because if not, then our dependency would not be active); or the dependency is required,
                 // meaning that either the service is not yet started, or already started.
@@ -479,7 +511,10 @@ public class ServiceDependencyImpl exten
                 
                 // we only try to invoke the method here if we are really already instantiated
                 if (ds.isInstantiated() && ds.getCompositionInstances().length > 0) {
-                    invokeAdded(ds, ref, service);
+                	if (m_debug) {
+                		log("invoke added: " + ref);
+                	}
+                    invokeAdded(ds, ref, service); //**
                 }
             }
         }
@@ -500,8 +535,13 @@ public class ServiceDependencyImpl exten
     }
 
     public void removedService(ServiceReference ref, Object service) {
+    	if (m_debug) {
+    		log("removedservice: " + ref);
+    	}
         boolean makeUnavailable = makeUnavailable();
-        
+        if (m_debug) {
+        	log("make unavailable: " + makeUnavailable);
+        }
         Object[] services;
         synchronized (this) {
             services = m_services.toArray();
@@ -511,6 +551,8 @@ public class ServiceDependencyImpl exten
             DependencyService ds = (DependencyService) services[i];
             if (makeUnavailable) {
                 ds.dependencyUnavailable(this);
+                // when the dependency is optional or the dependency is instance bound and the component is instantiated (and the dependency is required)
+                // then remove is invoked. In other cases the removed has been when the component was unconfigured.
                 if (!isRequired() || (ds.isInstantiated() && isInstanceBound())) {
                     invokeRemoved(ds, ref, service);
                 }
@@ -526,6 +568,9 @@ public class ServiceDependencyImpl exten
     }
     
     public void invokeAdded(DependencyService dependencyService, ServiceReference reference, Object service) {
+    	if (m_debug) {
+    		log("invoke added");
+    	}
         boolean added = false;
         synchronized (m_sr) {
             Set set = (Set) m_sr.get(dependencyService);
@@ -546,14 +591,39 @@ public class ServiceDependencyImpl exten
         }
     }
     
-    private void handleAspectAwareAdded(DependencyService dependencyService, ServiceReference reference, Object service) {
+    private synchronized void waitForCallbackLock(Runnable runnable) {
+    	while (m_injectionQueue.indexOf(runnable) != 0) {
+    		try {
+				wait();
+			} catch (InterruptedException e) {
+			}
+    	}
+    }
+    
+    private synchronized void enqueueCallback(Runnable runnable) {
+    	m_injectionQueue.addLast(runnable);
+    }
+    
+    private synchronized void releaseCallback(Runnable runnable) {
+    	m_injectionQueue.remove(runnable);
+    	notifyAll();
+    }
+    
+    private void handleAspectAwareAdded(final DependencyService dependencyService, final ServiceReference reference, final Object service) {
+    	if (m_debug) {
+    		log("aspectawareadded: " + reference.getProperty("service.ranking"));
+    	}
 		if (componentIsDependencyManagerFactory(dependencyService)) {
 			// component is either aspect or adapter factory instance, these must be ignored.
 			return;
 		}
 		boolean invokeAdded = false;
+		boolean invokeSwapped = false;
 		Integer ranking = ServiceUtil.getRankingAsInteger(reference);
-		Tuple highestRankedService = null;
+		Tuple newHighestRankedService = null;
+		Tuple prevHighestRankedService = null;
+		Runnable callbackRunnable = null;
+		Map rankings = null;
 		synchronized (m_componentByRank) {
 			Long originalServiceId = ServiceUtil.getServiceIdAsLong(reference);
 			Map componentMap = (Map) m_componentByRank.get(dependencyService); /* <Long, Map<Integer, Tuple>> */
@@ -562,7 +632,7 @@ public class ServiceDependencyImpl exten
 				componentMap = new HashMap(); /* <Long, Map<Integer, Tuple>> */
 				m_componentByRank.put(dependencyService, componentMap);
 			}
-			Map rankings = (Map) componentMap.get(originalServiceId); /* <Integer, Tuple> */
+			rankings = (Map) componentMap.get(originalServiceId); /* <Integer, Tuple> */
 			if (rankings == null) {
 				// new component added
 				rankings = new HashMap(); /* <Integer, Tuple> */
@@ -572,14 +642,84 @@ public class ServiceDependencyImpl exten
 			} 
 			
 			if (!invokeAdded) {
-				highestRankedService = swapHighestRankedService(dependencyService, originalServiceId, reference, service, ranking);
+				// current highest ranked
+				prevHighestRankedService = (Tuple)getHighestRankedService(dependencyService, originalServiceId).getValue();
+				newHighestRankedService = swapHighestRankedService(dependencyService, originalServiceId, reference, service, ranking);
+				if (m_debug) {
+					log("prevhigh: " + prevHighestRankedService.getServiceReference().getProperty("service.ranking") + ", new high: " + newHighestRankedService.getServiceReference().getProperty("service.ranking"));
+				}
+				if (!prevHighestRankedService.getServiceReference().equals(newHighestRankedService.getServiceReference())) {
+					// new highest ranked service
+					if (m_debug) {
+						log("New highest ranked to swap to");
+					}
+					invokeSwapped = true;
+				} else {
+					if (m_debug) {
+						log("Ignoring lower ranked or irrelevant swap");
+					}
+				}
+			}
+			if (m_debug) {
+				log(m_componentByRank.toString());
+			}
+			
+			// up until this point should be synchronized on m_componentsByRank to keep integrity of the administration and consequences
+			// then the do phase comes, here we want to guarantee the effects of this operation are done like they were synchronized, however
+			// synchronization on m_componentsByRank to too course grained here, so we'd like to switch to synchronization on the
+			// original service id, therefore we're using our own guarded block to ensure the correct order.
+			
+			if (invokeAdded) {
+				if (m_debug) {
+					log("invoke added: " + reference.getProperty("service.ranking"));
+				}
+				callbackRunnable = createCallbackRunnable(dependencyService, reference, service, m_callbackAdded);
+				enqueueCallback(callbackRunnable);
+			} else if (invokeSwapped) {
+				if (m_debug) {
+					log("invoke swapped: " + newHighestRankedService.getServiceReference().getProperty("service.ranking") + " replacing " + prevHighestRankedService.getServiceReference().getProperty("service.ranking"));
+				}
+				callbackRunnable = createSwapRunnable(dependencyService, prevHighestRankedService.getServiceReference(), prevHighestRankedService.getService(), newHighestRankedService.getServiceReference(), newHighestRankedService.getService());
+				enqueueCallback(callbackRunnable);
+			}    	
+		}
+		if (callbackRunnable != null) {
+			waitForCallbackLock(callbackRunnable);
+			synchronized (rankings) {
+				releaseCallback(callbackRunnable);
+				execute(callbackRunnable);
 			}
 		}
-		if (invokeAdded) {
-			invoke(dependencyService, reference, service, m_callbackAdded);
-		} else {
-			invokeSwappedCallback(dependencyService, highestRankedService.getServiceReference(), highestRankedService.getService(), reference, service);
-		}    	
+    }
+    
+    private Runnable createCallbackRunnable(final DependencyService dependencyService, final ServiceReference reference, final Object service, final String callback) {
+    	return new Runnable() {
+			public void run() {
+				invoke(dependencyService, reference, service, callback);			
+			}
+			public String toString() {
+				return callback + " on " + dependencyService;
+			}
+		};
+    }
+    
+    private Runnable createSwapRunnable(final DependencyService dependencyService, final ServiceReference prevReference, final Object prevService, final ServiceReference newReference, final Object newService) {
+    	return new Runnable() {
+			public void run() {
+				invokeSwappedCallback(dependencyService, prevReference, prevService, newReference, newService);					
+			}
+			public String toString() {
+				return "swap on " + dependencyService;
+			}
+		};
+    }
+    
+    private void execute(Runnable runnable) {
+    	runnable.run();
+    }
+    
+    private void log(String message) {
+    	m_logger.log(Logger.LOG_DEBUG, message);
     }
     
     private boolean componentIsDependencyManagerFactory(DependencyService dependencyService) {
@@ -594,13 +734,12 @@ public class ServiceDependencyImpl exten
     }
     
 	private Tuple swapHighestRankedService(DependencyService dependencyService, Long serviceId, ServiceReference newReference, Object newService, Integer newRanking) {
-		// does a component with a higher ranking exists
+		// does a component with a higher ranking exist
 		synchronized (m_componentByRank) {
 			Map componentMap = (Map) m_componentByRank.get(dependencyService); /* <Long, Map<Integer, Tuple>> */
 			Map rankings = (Map) componentMap.get(serviceId); /* <Integer, Tuple> */
-			Entry highestEntry = getHighestRankedService(dependencyService, serviceId); /* <Integer, Tuple> */
-			rankings.remove(highestEntry.getKey());
 			rankings.put(newRanking, new Tuple(newReference, newService));
+			Entry highestEntry = getHighestRankedService(dependencyService, serviceId); /* <Integer, Tuple> */
 			return (Tuple) highestEntry.getValue();
 		}
 	}
@@ -643,6 +782,9 @@ public class ServiceDependencyImpl exten
 		// 1) a call to added(original-service)
 		// 2) that causes a swap
 		// 3) a call to removed(aspect-service) <-- that's what we're talking about
+		if (m_debug) {
+			log(m_componentByRank.toString());
+		}
 		return (componentMap != null && rankings != null && rankings.size() == 1 && ((Entry)rankings.entrySet().iterator().next()).getKey()
 				.equals(ServiceUtil.getRankingAsInteger(reference)));
 	}
@@ -653,11 +795,17 @@ public class ServiceDependencyImpl exten
     }
 
     public void invokeRemoved(DependencyService dependencyService, ServiceReference reference, Object service) {
+    	if (m_debug) {
+    		log("invoke removed");
+    	}
         boolean removed = false;
         synchronized (m_sr) {
             Set set = (Set) m_sr.get(dependencyService);
             removed = (set != null && set.remove(new Tuple(reference, service)));
         }
+        if (m_debug) {
+        	log("removed: " + removed);
+        }
         if (removed) {
         	if (m_callbackSwapped != null) {
         		handleAspectAwareRemoved(dependencyService, reference, service);
@@ -669,37 +817,94 @@ public class ServiceDependencyImpl exten
     }
     
 	private void handleAspectAwareRemoved(DependencyService dependencyService, ServiceReference reference, Object service) {
+		if (m_debug) {
+			log("aspectawareremoved: " + reference.getProperty("service.ranking"));
+		}
 		if (componentIsDependencyManagerFactory(dependencyService)) {
 			// component is either aspect or adapter factory instance, these must be ignored.
 			return;
 		}
+		// we might need to swap here too!
+		boolean invokeRemoved = false;
 		Long serviceId = ServiceUtil.getServiceIdAsLong(reference);
-			synchronized (m_componentByRank) {
-				if (isLastService(dependencyService, reference, service, serviceId)) {
-					invoke(dependencyService, reference, service, m_callbackRemoved);
-				}
-				Long originalServiceId = ServiceUtil.getServiceIdAsLong(reference);
-				Map componentMap = (Map) m_componentByRank.get(dependencyService); /* <Long, Map<Integer, Tuple>> */
-				if (componentMap != null) {
-					Map rankings = (Map) componentMap.get(originalServiceId); /* <Integer, Tuple> */
-					for (Iterator entryIterator = rankings.entrySet().iterator(); entryIterator.hasNext(); ) {
-						Entry mapEntry = (Entry) entryIterator.next();
-						if (((Tuple)mapEntry.getValue()).getServiceReference().equals(reference)) {
-							// remove the reference
-							rankings.remove(mapEntry.getKey());
-						}
-					}
-					if (rankings.size() == 0) {
-						componentMap.remove(originalServiceId);
+		Tuple prevHighestRankedService = null;
+		Tuple newHighestRankedService = null;
+		boolean invokeSwapped = false;
+		Map rankings = null;
+		Runnable callbackRunnable = null;
+		synchronized (m_componentByRank) {
+			Long originalServiceId = ServiceUtil.getServiceIdAsLong(reference);
+			if (isLastService(dependencyService, reference, service, serviceId)) {
+				invokeRemoved = true;
+			} else {
+				// not the last service, but should we swap?
+				prevHighestRankedService = (Tuple)getHighestRankedService(dependencyService, originalServiceId).getValue();
+				if (prevHighestRankedService.getServiceReference().equals(reference)) {
+					// swapping out
+					if (m_debug) {
+						log("Swap out on remove!");
 					}
-					if (componentMap.size() == 0) {
-						m_componentByRank.remove(dependencyService);
+					invokeSwapped = true;
+				}
+			}
+			if (m_debug) {
+				log("is last service: " + invokeRemoved);
+			}
+			// cleanup
+			Map componentMap = (Map) m_componentByRank.get(dependencyService); /* <Long, Map<Integer, Tuple>> */
+			if (componentMap != null) {
+				rankings = (Map) componentMap.get(originalServiceId); /* <Integer, Tuple> */
+				List rankingsToRemove = new ArrayList();
+				for (Iterator entryIterator = rankings.entrySet().iterator(); entryIterator.hasNext(); ) {
+					Entry mapEntry = (Entry) entryIterator.next();
+					if (((Tuple)mapEntry.getValue()).getServiceReference().equals(reference)) {
+						// remove the reference
+						// rankings.remove(mapEntry.getKey());
+						rankingsToRemove.add(mapEntry.getKey());
 					}
 				}
+				for (Iterator rankingIterator = rankingsToRemove.iterator(); rankingIterator.hasNext(); ) {
+					rankings.remove(rankingIterator.next());
+				}
+				if (rankings.size() == 0) {
+					componentMap.remove(originalServiceId);
+				}
+				if (componentMap.size() == 0) {
+					m_componentByRank.remove(dependencyService);
+				}
+			}
+			// determine current highest ranked service
+			if (invokeSwapped) {
+				newHighestRankedService = (Tuple)getHighestRankedService(dependencyService, originalServiceId).getValue();
+			}
+			if (invokeRemoved) {
+				// handle invoke outside the sync block since we won't know what will happen there
+				if (m_debug) {
+					log("invoke removed: " + reference.getProperty("service.ranking"));
+				}
+				callbackRunnable = createCallbackRunnable(dependencyService, reference, service, m_callbackRemoved);
+				enqueueCallback(callbackRunnable);
+			} else if (invokeSwapped) {
+				if (m_debug) {
+					log("invoke swapped: " + newHighestRankedService.getServiceReference().getProperty("service.ranking") + " replacing " + prevHighestRankedService.getServiceReference().getProperty("service.ranking"));
+				}
+				callbackRunnable = createSwapRunnable(dependencyService, prevHighestRankedService.getServiceReference(), prevHighestRankedService.getService(), newHighestRankedService.getServiceReference(), newHighestRankedService.getService());
+				enqueueCallback(callbackRunnable);
+			}
+		}
+		if (callbackRunnable != null) {
+			waitForCallbackLock(callbackRunnable);
+			synchronized (rankings) {
+				releaseCallback(callbackRunnable);
+				execute(callbackRunnable);
 			}
+		}
 	}    
 
     public void invoke(DependencyService dependencyService, ServiceReference reference, Object service, String name) {
+    	if (m_debug) {
+    		log("invoke: " + name);
+    	}
         if (name != null) {
             dependencyService.invokeCallbackMethod(getCallbackInstances(dependencyService), name, 
                 new Class[][] {
@@ -724,11 +929,26 @@ public class ServiceDependencyImpl exten
 			throw new IllegalStateException("Attempt to swap a service for a service with the same rank! previousReference: " + previousReference + ", currentReference: " + currentServiceReference);
 		}
 		
-		component.invokeCallbackMethod(getCallbackInstances(component), m_callbackSwapped, new Class[][] { { m_trackedServiceName, m_trackedServiceName },
-				{ Object.class, Object.class }, { ServiceReference.class, m_trackedServiceName, ServiceReference.class, m_trackedServiceName },
-				{ ServiceReference.class, Object.class, ServiceReference.class, Object.class } }, new Object[][] { { previous, current },
-				{ previous, current }, { previousReference, previous, currentServiceReference, current },
-				{ previousReference, previous, currentServiceReference, current } });
+		component.invokeCallbackMethod(getCallbackInstances(component), m_callbackSwapped, new Class[][] { 
+					{ m_trackedServiceName, m_trackedServiceName },
+					{ Object.class, Object.class }, 
+					{ ServiceReference.class, m_trackedServiceName, ServiceReference.class, m_trackedServiceName },
+					{ ServiceReference.class, Object.class, ServiceReference.class, Object.class }, 
+					{ Component.class, m_trackedServiceName, m_trackedServiceName },
+					{ Component.class, Object.class, Object.class }, 
+					{ Component.class, ServiceReference.class, m_trackedServiceName, ServiceReference.class, m_trackedServiceName },
+					{ Component.class, ServiceReference.class, Object.class, ServiceReference.class, Object.class } 
+				}, 
+				new Object[][] { 
+					{ previous, current },
+					{ previous, current }, 
+					{ previousReference, previous, currentServiceReference, current },
+					{ previousReference, previous, currentServiceReference, current },
+					{ component, previous, current },
+					{ component, previous, current }, 
+					{ component, previousReference, previous, currentServiceReference, current },
+					{ component, previousReference, previous, currentServiceReference, current } 					
+				});
 	}    
 
     protected synchronized boolean makeAvailable() {
@@ -1066,6 +1286,9 @@ public class ServiceDependencyImpl exten
     }
 
     public void invokeAdded(DependencyService service) {
+    	if (m_debug) {
+    		log("invoke added due to configure. (component is activated)");
+    	}
         ServiceReference[] refs = m_tracker.getServiceReferences();
         if (refs != null) {
             for (int i = 0; i < refs.length; i++) {
@@ -1077,20 +1300,26 @@ public class ServiceDependencyImpl exten
     }
     
     public void invokeRemoved(DependencyService service) {
+    	if (m_debug) {
+    		log("invoke removed due to unconfigure. (component is destroyed)");
+    	}
         Set references = null;
+        Object[] tupleArray = null;
         synchronized (m_sr) {
             references = (Set) m_sr.get(service);
+            // is this null check necessary ??
+            if (references != null) {
+	            tupleArray = references.toArray(new Tuple[references.size()]);
+            }
         }
-        Tuple[] refs = (Tuple[]) (references != null ? references.toArray(new Tuple[references.size()]) : new Tuple[0]);
+
+        Tuple[] refs = (Tuple[]) (tupleArray != null ?  tupleArray : new Tuple[0]);
     
         for (int i = 0; i < refs.length; i++) {
             ServiceReference sr = refs[i].getServiceReference();
             Object svc = refs[i].getService();
             invokeRemoved(service, sr, svc);
         }
-        if (references != null) {
-            references.clear();
-        }
     }
 
     public Dictionary getProperties() {

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/BundleContextInterceptor.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/BundleContextInterceptor.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/BundleContextInterceptor.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/BundleContextInterceptor.java Mon Jun  3 12:13:09 2013
@@ -21,8 +21,10 @@ package org.apache.felix.dm.impl.index;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.felix.dm.FilterIndex;
+import org.apache.felix.dm.impl.Logger;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Constants;
 import org.osgi.framework.InvalidSyntaxException;
@@ -34,11 +36,16 @@ import org.osgi.framework.ServiceReferen
  * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
  */
 public class BundleContextInterceptor extends BundleContextInterceptorBase {
+	private static final String INDEX_LOG_BAD_PERFORMING_FILTERS = "org.apache.felix.dependencymanager.index.logbadperformingfilters";
+	private static AtomicLong maxLookupTime = new AtomicLong(0L);
     private final ServiceRegistryCache m_cache;
+    private final boolean perfmon = "true".equals(System.getProperty(INDEX_LOG_BAD_PERFORMING_FILTERS));
+	private final Logger m_logger;
 
-    public BundleContextInterceptor(ServiceRegistryCache cache, BundleContext context) {
+    public BundleContextInterceptor(ServiceRegistryCache cache, BundleContext context, Logger logger) {
         super(context);
         m_cache = cache;
+		m_logger = logger;
     }
 
     public void addServiceListener(ServiceListener listener, String filter) throws InvalidSyntaxException {
@@ -74,6 +81,10 @@ public class BundleContextInterceptor ex
     }
 
     public ServiceReference[] getServiceReferences(String clazz, String filter) throws InvalidSyntaxException {
+    	long start = 0L;
+    	if (perfmon) {
+    		start = System.currentTimeMillis();
+    	}
         // first we ask the cache if there is an index for our request (class and filter combination)
         FilterIndex filterIndex = m_cache.hasFilterIndexFor(clazz, filter);
         if (filterIndex != null) {
@@ -89,6 +100,13 @@ public class BundleContextInterceptor ex
                     }
                 }
             }
+            if (perfmon) {
+	        	long duration = System.currentTimeMillis() - start;
+	        	if (maxLookupTime.get() < duration) {
+	        		maxLookupTime.set(duration);
+	        		m_logger.log(org.apache.felix.dm.impl.Logger.LOG_DEBUG, "new worst performing filter (" + duration + "ms.): " + clazz + " " + filter);
+	        	}
+            }
             if (result == null || result.size() == 0) {
                 return null;
             }
@@ -96,7 +114,15 @@ public class BundleContextInterceptor ex
         }
         else {
             // if they don't know, we ask the real bundle context instead
-            return m_context.getServiceReferences(clazz, filter);
+            ServiceReference[] serviceReferences = m_context.getServiceReferences(clazz, filter);
+            if (perfmon) {
+	        	long duration = System.currentTimeMillis() - start;
+	        	if (maxLookupTime.get() < duration) {
+	        		maxLookupTime.set(duration);
+	        		System.out.println("new worst performing filter (" + duration + "ms.): " + clazz + " " + filter);
+	        	}
+            }
+        	return serviceReferences;
         }
     }
 

Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/ServiceRegistryCache.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/ServiceRegistryCache.java?rev=1488970&r1=1488969&r2=1488970&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/ServiceRegistryCache.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/ServiceRegistryCache.java Mon Jun  3 12:13:09 2013
@@ -18,6 +18,7 @@
  */
 package org.apache.felix.dm.impl.index;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -25,6 +26,7 @@ import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.apache.felix.dm.FilterIndex;
+import org.apache.felix.dm.impl.Logger;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceEvent;
 import org.osgi.framework.ServiceListener;
@@ -34,7 +36,8 @@ import org.osgi.framework.ServiceRegistr
  * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
  */
 public class ServiceRegistryCache implements ServiceListener/*, CommandProvider*/ {
-    private final List /* <FilterIndex> */ m_filterIndexList = new CopyOnWriteArrayList();
+    private static final String INDEX_PERFLOG = "org.apache.felix.dependencymanager.index.logmissingindices";
+	private final List /* <FilterIndex> */ m_filterIndexList = new CopyOnWriteArrayList();
     private final BundleContext m_context;
     private final FilterIndexBundleContext m_filterIndexBundleContext;
     private final Map /* <BundleContext, BundleContextInterceptor> */ m_bundleContextInterceptorMap = new HashMap();
@@ -42,10 +45,16 @@ public class ServiceRegistryCache implem
     private long m_arrayVersion = -1;
     private BundleContextInterceptor[] m_interceptors = null;
     private ServiceRegistration m_registration;
-
+    private boolean m_dumpUnIndexedFilters = "true".equals(System.getProperty(INDEX_PERFLOG));
+    private List m_unindexedFilters = new ArrayList();
+	private Logger m_logger;
     
     public ServiceRegistryCache(BundleContext context) {
         m_context = context;
+        // only obtain the logservice when we actually want to log something.
+        if (System.getProperty(INDEX_PERFLOG) != null) {
+        	m_logger = new Logger(context);
+        }
         m_filterIndexBundleContext = new FilterIndexBundleContext(m_context);
     }
     
@@ -89,7 +98,7 @@ public class ServiceRegistryCache implem
         synchronized (m_bundleContextInterceptorMap) {
             BundleContextInterceptor bundleContextInterceptor = (BundleContextInterceptor) m_bundleContextInterceptorMap.get(context);
             if (bundleContextInterceptor == null) {
-                bundleContextInterceptor = new BundleContextInterceptor(this, context);
+                bundleContextInterceptor = new BundleContextInterceptor(this, context, m_logger);
                 m_bundleContextInterceptorMap.put(context, bundleContextInterceptor);
                 m_currentVersion++;
                 // TODO figure out a good way to clean up bundle contexts that are no longer valid so they can be garbage collected
@@ -106,6 +115,13 @@ public class ServiceRegistryCache implem
                 return filterIndex;
             }
         }
+        if (m_dumpUnIndexedFilters) {
+        	String filterStr = clazz + ":" + filter;
+	        if (!m_unindexedFilters.contains(filterStr)) {
+	        	m_unindexedFilters.add(filterStr);
+	        	m_logger.log(Logger.LOG_DEBUG, "No filter index for " + filterStr);
+	        }
+        }
         return null;
     }
 

Copied: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Filter.java (from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Filter.java)
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Filter.java?p2=felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Filter.java&p1=felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Filter.java&r1=1487101&r2=1488970&rev=1488970&view=diff
==============================================================================
--- felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Filter.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Filter.java Mon Jun  3 12:13:09 2013
@@ -1,4 +1,22 @@
-package org.apache.felix.dm.impl.index;
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.dm.impl.index.multiproperty;
 
 import java.util.HashMap;
 import java.util.Iterator;
@@ -7,17 +25,20 @@ import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
 
+/**
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
 public class Filter {
 	
-	private boolean valid = true;
-	private Map /* <String, Property> */ properties = new HashMap();
-	private Set /* <String> */ propertyKeys = new TreeSet(String.CASE_INSENSITIVE_ORDER);
+	private boolean m_valid = true;
+	private Map /* <String, Property> */ m_properties = new HashMap();
+	private Set /* <String> */ m_propertyKeys = new TreeSet(String.CASE_INSENSITIVE_ORDER);
 	
 	private Filter() {
 		
 	}
 	
-	// (&(objectClass=OBJECTCLASS)(&(model=MODEL)(concept=CONCEPT)(role=ROLE)(!(context=*))))
+	// Sample valid filter string (&(objectClass=OBJECTCLASS)(&(model=MODEL)(concept=CONCEPT)(role=ROLE)(!(context=*))))
 	public static Filter parse(String filterString) {
 		Filter filter = new Filter();
 		StringTokenizer tokenizer = new StringTokenizer(filterString, "(&|=)", true);
@@ -33,7 +54,7 @@ public class Filter {
 			token = tokenizer.nextToken();
 			if (token.equals("|")) {
 				// we're not into OR's
-				filter.valid = false;
+				filter.m_valid = false;
 				break;
 			}
 			if (token.equals("!")) {
@@ -46,14 +67,14 @@ public class Filter {
 				}
 				if (token.equals(")")) {
 					// set complete
-					if (filter.properties.containsKey(key)) {
+					if (filter.m_properties.containsKey(key)) {
 						// set current property to multivalue
-						Property property = (Property) filter.properties.get(key);
-						property.addValue(valueBuilder.toString());
+						Property property = (Property) filter.m_properties.get(key);
+						property.addValue(valueBuilder.toString(), negate);
 					} else {
 						Property property = new Property(negate, key, valueBuilder.toString());
-						filter.properties.put(key, property);
-						filter.propertyKeys.add(key);
+						filter.m_properties.put(key, property);
+						filter.m_propertyKeys.add(key);
 					}
 					negate = false;
 					key = null;
@@ -65,19 +86,31 @@ public class Filter {
 	}
 	
 	public boolean containsProperty(String propertyKey) {
-		return properties.containsKey(propertyKey);
+		return m_properties.containsKey(propertyKey);
 	}
 	
 	public Set /* <String> */ getPropertyKeys() {
-		return properties.keySet();
+		return m_properties.keySet();
 	}
 	
 	public Property getProperty(String key) {
-		return (Property) properties.get(key);
+		return (Property) m_properties.get(key);
 	}
 	
 	public boolean isValid() {
-		return valid;
+		if (!m_valid) {
+			return m_valid;
+		} else {
+			// also check the properties
+			Iterator propertiesIterator = m_properties.values().iterator();
+			while (propertiesIterator.hasNext()) {
+				Property property = (Property) propertiesIterator.next();
+				if (!property.isValid()) {
+					return false;
+				}
+			}
+		}
+		return true;
 	}
 	
 	public static void main(String args[]) {
@@ -87,11 +120,11 @@ public class Filter {
 
 	protected String createKey() {
 		StringBuilder builder = new StringBuilder();
-		Iterator keys = propertyKeys.iterator();
+		Iterator keys = m_propertyKeys.iterator();
 		
 		while (keys.hasNext()) {
 			String key = (String) keys.next();
-			Property prop = (Property) properties.get(key);
+			Property prop = (Property) m_properties.get(key);
 			if (!prop.isWildcard()) {
 				Iterator values = prop.getValues().iterator();
 				while (values.hasNext()) {

Copied: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/MultiPropertyFilterIndex.java (from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/AdvancedMultiPropertyFilterIndex.java)
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/MultiPropertyFilterIndex.java?p2=felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/MultiPropertyFilterIndex.java&p1=felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/AdvancedMultiPropertyFilterIndex.java&r1=1487101&r2=1488970&rev=1488970&view=diff
==============================================================================
--- felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/AdvancedMultiPropertyFilterIndex.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/MultiPropertyFilterIndex.java Mon Jun  3 12:13:09 2013
@@ -1,4 +1,22 @@
-package org.apache.felix.dm.impl.index;
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.dm.impl.index.multiproperty;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -22,18 +40,21 @@ import org.osgi.framework.ServiceEvent;
 import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceReference;
 
-public class AdvancedMultiPropertyFilterIndex implements FilterIndex, ServiceTrackerCustomizer {
+/**
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
+public class MultiPropertyFilterIndex implements FilterIndex, ServiceTrackerCustomizer {
 
     private final Object m_lock = new Object();
     private ServiceTracker m_tracker;
     private BundleContext m_context;
-	private Map /* <String, Property> */ configProperties = new LinkedHashMap();
-	private List /* <String> */ negatePropertyKeys = new ArrayList();
+	private Map /* <String, Property> */ m_configProperties = new LinkedHashMap();
+	private List /* <String> */ m_negatePropertyKeys = new ArrayList();
     private final Map /* <String, List<ServiceReference>> */ m_keyToServiceReferencesMap = new HashMap();
     private final Map /* <String, List<ServiceListener>> */ m_keyToListenersMap = new HashMap();
     private final Map /* <ServiceListener, String> */ m_listenerToFilterMap = new HashMap();
 
-	public AdvancedMultiPropertyFilterIndex(String configString) {
+	public MultiPropertyFilterIndex(String configString) {
 		parseConfig(configString);
 	}
 	
@@ -45,21 +66,21 @@ public class AdvancedMultiPropertyFilter
 		}
 		// compare property keys to the ones in the configuration
 		Set /* <String> */ filterPropertyKeys = filter.getPropertyKeys();
-		if (configProperties.size() != filterPropertyKeys.size()) {
+		if (m_configProperties.size() != filterPropertyKeys.size()) {
 			return false;
 		}
 		Iterator filterPropertyKeysIterator = filterPropertyKeys.iterator();
 		while (filterPropertyKeysIterator.hasNext()) {
 			String filterPropertyKey = (String) filterPropertyKeysIterator.next();
-			if (!configProperties.containsKey(filterPropertyKey)) {
+			if (!m_configProperties.containsKey(filterPropertyKey)) {
 				return false;
-			} else if (((Property)configProperties.get(filterPropertyKey)).isNegate() != filter.getProperty(filterPropertyKey).isNegate()) {
+			} else if (((Property)m_configProperties.get(filterPropertyKey)).isNegate() != filter.getProperty(filterPropertyKey).isNegate()) {
 				// negation should be equal
 				return false;
 			} else if (!filter.getProperty(filterPropertyKey).isNegate() && filter.getProperty(filterPropertyKey).getValue().equals("*")) {
 				// no wildcards without negation allowed
 				return false;
-			}
+			} 
 		}
 		// our properties match so we're applicable
 		return true;
@@ -71,10 +92,10 @@ public class AdvancedMultiPropertyFilter
         for (int i = 0; i < propertyKeys.length; i++) {
             referenceProperties.add(propertyKeys[i]);
         }
-        Iterator iterator = configProperties.keySet().iterator();
+        Iterator iterator = m_configProperties.keySet().iterator();
         while (iterator.hasNext()) {
             String item = (String) iterator.next();
-            Property configProperty = (Property) configProperties.get(item);
+            Property configProperty = (Property) m_configProperties.get(item);
             if (!configProperty.isNegate() && !(referenceProperties.contains(item))) {
                 return false;
             } else if (configProperty.isNegate() && referenceProperties.contains(item)) {
@@ -102,16 +123,16 @@ public class AdvancedMultiPropertyFilter
 				value = "*";
 			}
 			property.setKey(key.toLowerCase());
-			property.addValue(value);
-			configProperties.put(key.toLowerCase(), property);
+			property.addValue(value, property.isNegate());
+			m_configProperties.put(key.toLowerCase(), property);
 			if (property.isNegate()) {
-				negatePropertyKeys.add(key);
+				m_negatePropertyKeys.add(key);
 			}
 		}
 	}
 	
 	protected Collection /* <Property> */ getProperties() {
-		return configProperties.values();
+		return m_configProperties.values();
 	}
 	
     protected String createKeyFromFilter(String clazz, String filterString) {
@@ -141,7 +162,7 @@ public class AdvancedMultiPropertyFilter
     	for (int i = 0; i < keys.length; i++) {
     		List set = new ArrayList();
     		String key = keys[i].toLowerCase();
-    		if (configProperties.containsKey(key)) {
+    		if (m_configProperties.containsKey(key)) {
 	    		Object valueObject = reference.getProperty(key);
 	    		if (valueObject instanceof String[]) {
 	    			set.addAll(getPermutations(key, (String[]) valueObject));
@@ -175,6 +196,14 @@ public class AdvancedMultiPropertyFilter
     	return results;
     }
     
+    /**
+     * Note that we calculate the carthesian product for multi value properties. Use filters on these sparingly since memory
+     * consumption can get really high when multiple properties have a lot of values.
+     * 
+     * @param index
+     * @param sets
+     * @return
+     */
     private List carthesianProduct(int index, List sets) {
     	List result = new ArrayList();
     	if (index == sets.size()) {
@@ -331,7 +360,7 @@ public class AdvancedMultiPropertyFilter
     
     protected boolean shouldBeIndexed(ServiceReference reference) {
     	// is already applicable, so we should only check whether there's a negate field in the filter which has a value in the reference
-    	Iterator negatePropertyKeyIterator = negatePropertyKeys.iterator();
+    	Iterator negatePropertyKeyIterator = m_negatePropertyKeys.iterator();
     	while (negatePropertyKeyIterator.hasNext()) {
     		String negatePropertyKey = (String) negatePropertyKeyIterator.next();
     		if (reference.getProperty(negatePropertyKey) != null) {
@@ -447,7 +476,7 @@ public class AdvancedMultiPropertyFilter
     
     public String toString() {
         StringBuffer sb = new StringBuffer();
-        sb.append("MultiPropertyExactFilter[");
+        sb.append(" dMultiPropertyExactFilter[");
         sb.append("K2L: " + m_keyToListenersMap.size());
         sb.append(", K2SR: " + m_keyToServiceReferencesMap.size());
         sb.append(", L2F: " + m_listenerToFilterMap.size());

Copied: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Property.java (from r1487101, felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Property.java)
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Property.java?p2=felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Property.java&p1=felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Property.java&r1=1487101&r2=1488970&rev=1488970&view=diff
==============================================================================
--- felix/sandbox/uiterlix/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/Property.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/index/multiproperty/Property.java Mon Jun  3 12:13:09 2013
@@ -1,66 +1,99 @@
-package org.apache.felix.dm.impl.index;
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.dm.impl.index.multiproperty;
 
 import java.util.Set;
 import java.util.TreeSet;
 
-class Property {
-	boolean negate;
-	String key;
-	String value;
-	Set values = new TreeSet(String.CASE_INSENSITIVE_ORDER);
+/**
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
+import java.util.Set;
+import java.util.TreeSet;
+
+public class Property {
+	boolean m_negate;
+	boolean m_valid = true;
+	String m_key;
+	String m_value;
+	Set m_values = new TreeSet(String.CASE_INSENSITIVE_ORDER);
 	
 	public Property() {
 	}
 	
 	public Property(boolean negate, String key, String value) {
 		super();
-		this.negate = negate;
-		this.key = key.toLowerCase();
-		this.values.add(value);
-		this.value = value;
+		this.m_negate = negate;
+		this.m_key = key.toLowerCase();
+		this.m_values.add(value);
+		this.m_value = value;
 	}
 
 	public void setNegate(boolean negate) {
-		this.negate = negate;
+		this.m_negate = negate;
 	}
 	
 	public void setKey(String key) {
-		this.key = key.toLowerCase();
+		this.m_key = key.toLowerCase();
 	}
 	
-	public void addValue(String value) {
-		if (this.value == null) {
-			this.value = value;
+	public void addValue(String value, boolean negate) {
+		if (this.m_negate != negate) {
+			// multiproperty with different negations, causes invalid configuration.
+			m_valid = false;
 		}
-		values.add(value);
+		if (this.m_value == null) {
+			this.m_value = value;
+		}
+		m_values.add(value);
 	}
 	
 	public boolean isNegate() {
-		return negate;
+		return m_negate;
 	}
 	
 	public String getKey() {
-		return key;
+		return m_key;
 	}
 	
 	public String getValue() {
-		return value;
+		return m_value;
 	}
 	
 	public Set getValues() {
-		return values;
+		return m_values;
 	}
 	
 	public boolean isWildcard() {
-		return "*".equals(value);
+		return "*".equals(m_value);
 	}
 	
 	public boolean isMultiValue() {
-		return values.size() > 1;
+		return m_values.size() > 1;
 	}
 
 	public String toString() {
-		return "Property [negate=" + negate + ", key=" + key + ", values="
-				+ values + "]";
+		return "Property [negate=" + m_negate + ", key=" + m_key + ", values="
+				+ m_values + "]";
+	}
+	
+	public boolean isValid() {
+		return m_valid;
 	}
 }
\ No newline at end of file



Mime
View raw message