You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by am...@apache.org on 2013/11/08 12:16:05 UTC
svn commit: r1539981 - in /cxf/dosgi/trunk/discovery/local/src:
main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java
test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java
Author: amichai
Date: Fri Nov 8 11:16:05 2013
New Revision: 1539981
URL: http://svn.apache.org/r1539981
Log:
Fix LocalDiscovery synchronization and little memory leak
Modified:
cxf/dosgi/trunk/discovery/local/src/main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java
cxf/dosgi/trunk/discovery/local/src/test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java
Modified: cxf/dosgi/trunk/discovery/local/src/main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/discovery/local/src/main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java?rev=1539981&r1=1539980&r2=1539981&view=diff
==============================================================================
--- cxf/dosgi/trunk/discovery/local/src/main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java (original)
+++ cxf/dosgi/trunk/discovery/local/src/main/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscovery.java Fri Nov 8 11:16:05 2013
@@ -110,17 +110,18 @@ public class LocalDiscovery implements B
return;
}
- listenerToFilters.put(endpointListener, filters);
- for (String filter : filters) {
- Collection<EndpointListener> listeners = filterToListeners.get(filter);
- if (listeners != null) {
+ synchronized (listenerToFilters) {
+ listenerToFilters.put(endpointListener, filters);
+ for (String filter : filters) {
+ Collection<EndpointListener> listeners = filterToListeners.get(filter);
+ if (listeners == null) {
+ listeners = new ArrayList<EndpointListener>();
+ filterToListeners.put(filter, listeners);
+ }
listeners.add(endpointListener);
- } else {
- List<EndpointListener> list = new ArrayList<EndpointListener>();
- list.add(endpointListener);
- filterToListeners.put(filter, list);
}
}
+
triggerCallbacks(filters, endpointListener);
}
@@ -131,17 +132,36 @@ public class LocalDiscovery implements B
* @param endpointListener
*/
void removeListener(EndpointListener endpointListener) {
- Collection<String> filters = listenerToFilters.remove(endpointListener);
- if (filters == null) {
- return;
+ synchronized (listenerToFilters) {
+ Collection<String> filters = listenerToFilters.remove(endpointListener);
+ if (filters == null) {
+ return;
+ }
+
+ for (String filter : filters) {
+ Collection<EndpointListener> listeners = filterToListeners.get(filter);
+ if (listeners != null) {
+ listeners.remove(endpointListener);
+ if (listeners.isEmpty()) {
+ filterToListeners.remove(filter);
+ }
+ }
+ }
}
+ }
- for (String filter : filters) {
- Collection<EndpointListener> endpointListeners = filterToListeners.get(filter);
- if (endpointListeners != null) {
- endpointListeners.remove(endpointListener);
+ private Map<String, Collection<EndpointListener>> getMatchingListeners(EndpointDescription endpoint) {
+ // return a copy of matched filters/listeners so that caller doesn't need to hold locks while triggering events
+ Map<String, Collection<EndpointListener>> matched = new HashMap<String, Collection<EndpointListener>>();
+ synchronized (listenerToFilters) {
+ for (Entry<String, Collection<EndpointListener>> entry : filterToListeners.entrySet()) {
+ String filter = entry.getKey();
+ if (Utils.matchFilter(bundleContext, filter, endpoint)) {
+ matched.put(filter, new ArrayList<EndpointListener>(entry.getValue()));
+ }
}
}
+ return matched;
}
public void shutDown() {
@@ -190,23 +210,24 @@ public class LocalDiscovery implements B
}
private void triggerCallbacks(EndpointDescription endpoint, boolean added) {
- for (Map.Entry<EndpointListener, Collection<String>> entry : listenerToFilters.entrySet()) {
- for (String match : entry.getValue()) {
- triggerCallbacks(entry.getKey(), match, endpoint, added);
+ for (Map.Entry<String, Collection<EndpointListener>> entry : getMatchingListeners(endpoint).entrySet()) {
+ String filter = entry.getKey();
+ for (EndpointListener listener : entry.getValue()) {
+ triggerCallbacks(listener, filter, endpoint, added);
}
}
}
- private void triggerCallbacks(EndpointListener endpointListener, String toMatch,
+ private void triggerCallbacks(EndpointListener endpointListener, String filter,
EndpointDescription endpoint, boolean added) {
- if (!Utils.matchFilter(bundleContext, toMatch, endpoint)) {
+ if (!Utils.matchFilter(bundleContext, filter, endpoint)) {
return;
}
if (added) {
- endpointListener.endpointAdded(endpoint, toMatch);
+ endpointListener.endpointAdded(endpoint, filter);
} else {
- endpointListener.endpointRemoved(endpoint, toMatch);
+ endpointListener.endpointRemoved(endpoint, filter);
}
}
Modified: cxf/dosgi/trunk/discovery/local/src/test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/discovery/local/src/test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java?rev=1539981&r1=1539980&r2=1539981&view=diff
==============================================================================
--- cxf/dosgi/trunk/discovery/local/src/test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java (original)
+++ cxf/dosgi/trunk/discovery/local/src/test/java/org/apache/cxf/dosgi/discovery/local/internal/LocalDiscoveryTest.java Fri Nov 8 11:16:05 2013
@@ -21,12 +21,10 @@ package org.apache.cxf.dosgi.discovery.l
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.Dictionary;
import java.util.HashSet;
import java.util.Hashtable;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -275,10 +273,9 @@ public class LocalDiscoveryTest extends
assertEquals(1, ld.listenerToFilters.size());
assertEquals(Arrays.asList("(|(objectClass=org.example.ClassA)(objectClass=org.example.ClassB))"),
ld.listenerToFilters.get(el));
- assertEquals(2, ld.filterToListeners.size());
+ assertEquals(1, ld.filterToListeners.size());
assertEquals(Collections.singletonList(el),
ld.filterToListeners.get("(|(objectClass=org.example.ClassA)(objectClass=org.example.ClassB))"));
- assertEquals(0, ld.filterToListeners.get("(objectClass=org.example.ClassA)").size());
EasyMock.verify(el);
Set<String> expectedEndpoints = new HashSet<String>(Arrays.asList("org.example.ClassA", "org.example.ClassB"));
@@ -287,10 +284,7 @@ public class LocalDiscoveryTest extends
// Remove the EndpointListener Service
ld.listenerTracker.removedService(sr, el);
assertEquals(0, ld.listenerToFilters.size());
- assertEquals(2, ld.filterToListeners.size());
- Iterator<Collection<EndpointListener>> valIter = ld.filterToListeners.values().iterator();
- assertEquals(0, valIter.next().size());
- assertEquals(0, valIter.next().size());
+ assertEquals(0, ld.filterToListeners.size());
}
public void testRegisterTracker() throws Exception {
@@ -390,8 +384,7 @@ public class LocalDiscoveryTest extends
assertEquals(1, ld.filterToListeners.values().iterator().next().size());
ld.removeListener(endpointListener);
assertEquals(0, ld.listenerToFilters.size());
- assertEquals(2, ld.filterToListeners.size());
- assertEquals(0, ld.filterToListeners.values().iterator().next().size());
+ assertEquals(0, ld.filterToListeners.size());
}
private LocalDiscovery getLocalDiscovery() throws InvalidSyntaxException {