Mercurial > hg > release > thermostat-1.6
changeset 1941:1da17b854973
Allow concurrent modifications of VmStatusListener's.
PR3047
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-June/019818.html
author | Jie Kang <jkang@redhat.com> |
---|---|
date | Fri, 24 Jun 2016 11:14:59 -0400 |
parents | 6c8f7bce641f |
children | 9db29a97e702 |
files | system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java |
diffstat | 2 files changed, 108 insertions(+), 1 deletions(-) [+] |
line wrap: on
line diff
--- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java Thu Jun 23 12:04:07 2016 -0400 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java Fri Jun 24 11:14:59 2016 -0400 @@ -41,6 +41,7 @@ import java.util.Map.Entry; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; @@ -63,7 +64,7 @@ private final Object listenerLock = new Object(); private final Map<Integer, String> activePids; - private final Map<VmStatusListener, Set<Integer>> listeners = new HashMap<>(); + private final Map<VmStatusListener, Set<Integer>> listeners = new ConcurrentHashMap<>(); private final ServiceTracker tracker;
--- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java Thu Jun 23 12:04:07 2016 -0400 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java Fri Jun 24 11:14:59 2016 -0400 @@ -36,10 +36,14 @@ package com.redhat.thermostat.backend.system; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import java.util.ConcurrentModificationException; + import org.junit.Test; +import org.osgi.framework.BundleContext; import com.redhat.thermostat.agent.VmStatusListener; import com.redhat.thermostat.agent.VmStatusListener.Status; @@ -96,5 +100,107 @@ verify(listener).vmStatusChanged(Status.VM_ACTIVE, VM_ID, VM_PID); } + + /* + * Some backends might activate on activation of another backend. If both of + * them are also VmStatusListener's, a concurrent modification exception + * might be thrown. This tests verifies it's OK to do so (concurrent modification). + */ + @Test + public void canAddListenersWhileFiringEvent() throws InterruptedException { + StubBundleContext bundleContext = new StubBundleContext(); + VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); + + // Add > 2 listeners. One of them registers another listener in vmStatusChanged() + // Thus provoking ConcurrentModificationException. + bundleContext.registerService(VmStatusListener.class, new TestVmStatusListener(bundleContext), null); + bundleContext.registerService(VmStatusListener.class, new VmStatusListener() { + + @Override + public void vmStatusChanged(Status newStatus, String vmId, + int pid) { + Debug.println("Second registered listener fired"); + } + + @Override + public int hashCode() { + return 2; // second listener to be fired + } + + }, null); + bundleContext.registerService(VmStatusListener.class, new VmStatusListener() { + + @Override + public void vmStatusChanged(Status newStatus, String vmId, + int pid) { + Debug.println("Third registered listener fired"); + } + + @Override + public int hashCode() { + return 3; + } + + }, null); + notifier.start(); + + try { + notifier.notifyVmStatusChange(Status.VM_STARTED, "foo-vmid", 333); + // this will trigger the newly added listener being invoked and counting + // down the latch. + notifier.notifyVmStatusChange(Status.VM_STARTED, "foo-other", 9999); + // pass + } catch (ConcurrentModificationException e) { + fail("Unexpected conncurrent modification exception!"); + } + } + + static class TestVmStatusListener implements VmStatusListener { + + private final BundleContext context; + + private TestVmStatusListener(BundleContext context) { + this.context = context; + } + + @Override + public void vmStatusChanged(Status newStatus, String vmId, int pid) { + Debug.println("First registered listener fired"); + context.registerService(VmStatusListener.class, new VmStatusListener() { + + @Override + public void vmStatusChanged(Status newStatus, String vmId, + int pid) { + Debug.println("Listener registered in listener (between first and second) fired"); + } + + @Override + public int hashCode() { + // Pick a large hash code since that tends to result in + // the listener being fired later on. + return 101; + } + + }, null); + } + + @Override + public int hashCode() { + // Heuristic for HashMap it tends to be picked first when iterating over + return 1; + } + + } + + static class Debug { + + static final boolean debugOn = false; // set to true for debug output + + static void println(String msg) { + if (debugOn) { + System.err.println(msg); + } + } + } }