From dae0da95e9d7d3d9ceeab0a4edf67346d6f1ed1c Mon Sep 17 00:00:00 2001 From: Dustin Johnson Date: Mon, 29 Apr 2013 09:26:45 -0500 Subject: [PATCH] Issue #1910 EventBus register/unregister fixes and test memory management Change-Id: I840b2ff12db8f24d3f5e186dff4e8a513a86f48e Former-commit-id: 33b21c4fafe929b22160103c963e5f13022f9613 [formerly 85ca4f57ecc534e81bf39dfb854e10593dd0a56d] [formerly 5101c1585d656999a9489f1a6ed7cb341835da36] [formerly f8bba72501c171fbaf42460238ae30269f538c25 [formerly 5101c1585d656999a9489f1a6ed7cb341835da36 [formerly eaf92d9fedd4a429bab3b19b9e61e044dde103eb]]] Former-commit-id: f8bba72501c171fbaf42460238ae30269f538c25 Former-commit-id: 549e7f64a3a83b47ebb715d61c377aef80f21a2a [formerly e51bd3ddcdb40d9df2e259f8a426cd23f8a723e7] Former-commit-id: 15658cb411b56c1e678ed37b04ddecc00e8b4a59 --- .../raytheon/uf/common/event/EventBus.java | 31 +++- .../EdexBandwidthContextFactory.java | 3 +- .../notification/BandwidthEventBus.java | 54 ++++++- .../AbstractBandwidthManagerIntTest.java | 25 ++- .../bandwidth/BandwidthManagerIntTest.java | 150 ++++++++++-------- .../services/validator/ValidatorImplTest.java | 1 + .../retrieval/handlers/RetrievalTaskTest.java | 7 + 7 files changed, 187 insertions(+), 84 deletions(-) rename tests/{unit => integration}/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java (99%) diff --git a/edexOsgi/com.raytheon.uf.common.event/src/com/raytheon/uf/common/event/EventBus.java b/edexOsgi/com.raytheon.uf.common.event/src/com/raytheon/uf/common/event/EventBus.java index a3c81b3ebe..7bb6a20771 100644 --- a/edexOsgi/com.raytheon.uf.common.event/src/com/raytheon/uf/common/event/EventBus.java +++ b/edexOsgi/com.raytheon.uf.common.event/src/com/raytheon/uf/common/event/EventBus.java @@ -2,6 +2,10 @@ package com.raytheon.uf.common.event; import java.util.ServiceLoader; +import com.raytheon.uf.common.status.IUFStatusHandler; +import com.raytheon.uf.common.status.UFStatus; +import com.raytheon.uf.common.status.UFStatus.Priority; + /** * The EventBus. * @@ -15,6 +19,7 @@ import java.util.ServiceLoader; * add unregister. * Dec 11, 2012 1407 djohnson Separate the creation of the Google EventBus from the wrapper class. * Feb 05, 2013 1580 mpduff Moved to common, use IEventBusHandler. + * Apr 29, 2013 1910 djohnson Watch for NPEs and errors unregistering. * * * @@ -29,6 +34,11 @@ public final class EventBus { .iterator().next(); } + private static final IUFStatusHandler statusHandler = UFStatus + .getHandler(EventBus.class); + + private static final String NULL_SUBSCRIBER = "Ignoring a null subscriber."; + private EventBus() { } @@ -40,7 +50,12 @@ public final class EventBus { * The subscriber to register */ public static void register(Object subscriber) { - handler.register(subscriber); + if (subscriber != null) { + handler.register(subscriber); + } else { + statusHandler.handle(Priority.WARN, NULL_SUBSCRIBER, + new IllegalArgumentException(NULL_SUBSCRIBER)); + } } /** @@ -50,7 +65,19 @@ public final class EventBus { * The subscriber to unregister */ public static void unregister(Object subscriber) { - handler.unregister(subscriber); + if (subscriber != null) { + try { + handler.unregister(subscriber); + } catch (Throwable t) { + statusHandler.handle(Priority.WARN, + "Unable to unregister subscriber of type [" + + subscriber.getClass().getName() + + "] from the retrieval event bus!", t); + } + } else { + statusHandler.handle(Priority.WARN, NULL_SUBSCRIBER, + new IllegalArgumentException(NULL_SUBSCRIBER)); + } } /** diff --git a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/EdexBandwidthContextFactory.java b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/EdexBandwidthContextFactory.java index 176c6309ee..ed6fce82b2 100644 --- a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/EdexBandwidthContextFactory.java +++ b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/EdexBandwidthContextFactory.java @@ -107,8 +107,7 @@ public class EdexBandwidthContextFactory implements BandwidthContextFactory { * @param instance * the {@link BandwidthManager} instance */ - @SuppressWarnings("unused") - private EdexBandwidthContextFactory(BandwidthManager instance) { + EdexBandwidthContextFactory(BandwidthManager instance) { EdexBandwidthContextFactory.instance = instance; this.bandwidthDao = null; this.bandwidthInitializer = null; diff --git a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/notification/BandwidthEventBus.java b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/notification/BandwidthEventBus.java index 085f5193ef..2bde94f13f 100644 --- a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/notification/BandwidthEventBus.java +++ b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/notification/BandwidthEventBus.java @@ -5,6 +5,9 @@ import java.util.ServiceLoader; import com.raytheon.uf.common.datadelivery.registry.DataSetMetaData; import com.raytheon.uf.common.datadelivery.registry.Subscription; import com.raytheon.uf.common.registry.event.RemoveRegistryEvent; +import com.raytheon.uf.common.status.IUFStatusHandler; +import com.raytheon.uf.common.status.UFStatus; +import com.raytheon.uf.common.status.UFStatus.Priority; import com.raytheon.uf.edex.datadelivery.bandwidth.dao.SubscriptionRetrieval; import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.SubscriptionRetrievalFulfilled; @@ -20,13 +23,14 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.SubscriptionRetriev * Oct 10, 2012 0726 djohnson Make buses final. * Dec 11, 2012 1286 djohnson Create a factory to hold Google event buses. * Feb 07, 2013 1543 djohnson Changed to behave similarly to EventBus. + * Apr 29, 2013 1910 djohnson Watch for NPEs and errors unregistering. * * * * @version 1.0 */ public class BandwidthEventBus { - + private static final BandwidthEventBusFactory eventBusFactory; static { eventBusFactory = ServiceLoader @@ -43,15 +47,25 @@ public class BandwidthEventBus { private static final com.google.common.eventbus.EventBus retrievalBus = eventBusFactory .getRetrievalBus(); + private static final IUFStatusHandler statusHandler = UFStatus + .getHandler(BandwidthEventBus.class); + + private static final String NULL_SUBSCRIBER = "Ignoring a null subscriber."; + /** * Registers an object with the event bus. * * @param subscriber */ public static void register(Object subscriber) { - BandwidthEventBus.retrievalBus.register(subscriber); - BandwidthEventBus.subscriptionBus.register(subscriber); - BandwidthEventBus.dataSetBus.register(subscriber); + if (subscriber != null) { + BandwidthEventBus.retrievalBus.register(subscriber); + BandwidthEventBus.subscriptionBus.register(subscriber); + BandwidthEventBus.dataSetBus.register(subscriber); + } else { + statusHandler.handle(Priority.WARN, NULL_SUBSCRIBER, + new IllegalArgumentException(NULL_SUBSCRIBER)); + } } /** @@ -60,9 +74,35 @@ public class BandwidthEventBus { * @param subscriber */ public static void unregister(Object subscriber) { - BandwidthEventBus.retrievalBus.unregister(subscriber); - BandwidthEventBus.subscriptionBus.unregister(subscriber); - BandwidthEventBus.dataSetBus.unregister(subscriber); + if (subscriber != null) { + try { + BandwidthEventBus.retrievalBus.unregister(subscriber); + } catch (Throwable t) { + statusHandler.handle(Priority.WARN, + "Unable to unregister subscriber of type [" + + subscriber.getClass().getName() + + "] from the retrieval event bus!", t); + } + try { + BandwidthEventBus.subscriptionBus.unregister(subscriber); + } catch (Throwable t) { + statusHandler.handle(Priority.WARN, + "Unable to unregister subscriber of type [" + + subscriber.getClass().getName() + + "] from the subscription event bus!", t); + } + try { + BandwidthEventBus.dataSetBus.unregister(subscriber); + } catch (Throwable t) { + statusHandler.handle(Priority.WARN, + "Unable to unregister subscriber of type [" + + subscriber.getClass().getName() + + "] from the dataSet event bus!", t); + } + } else { + statusHandler.handle(Priority.WARN, NULL_SUBSCRIBER, + new IllegalArgumentException(NULL_SUBSCRIBER)); + } } /** diff --git a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/AbstractBandwidthManagerIntTest.java b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/AbstractBandwidthManagerIntTest.java index a9564a46f8..de7746463a 100644 --- a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/AbstractBandwidthManagerIntTest.java +++ b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/AbstractBandwidthManagerIntTest.java @@ -63,6 +63,7 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.util.BandwidthUtil; * Feb 07, 2013 1543 djohnson Remove unnecessary test setup methods. * Feb 20, 2013 1543 djohnson Delegate to sub-classes for which route to create subscriptions for. * Mar 28, 2013 1841 djohnson Subscription is now UserSubscription. + * Apr 29, 2013 1910 djohnson Always shutdown bandwidth managers in tests. * * * @@ -136,12 +137,24 @@ public abstract class AbstractBandwidthManagerIntTest { @After public void tearDown() { PathManagerFactoryTest.initLocalization(); - try { - bandwidthManager.shutdown(); - } catch (IllegalArgumentException iae) { - // ignore any exceptions occurring about not being a registered - // event bus handler - iae.printStackTrace(); + shutdownBandwidthManager(bandwidthManager); + shutdownBandwidthManager(EdexBandwidthContextFactory.getInstance()); + new EdexBandwidthContextFactory(null); + } + + /** + * Shutdown the bandwidth manager safely. + * + * @param instance + */ + protected void shutdownBandwidthManager(BandwidthManager bwManager) { + if (bwManager != null) { + try { + bwManager.shutdown(); + } catch (IllegalArgumentException iae) { + // ignore any exceptions occurring about not being a registered + // event bus handler + } } } diff --git a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java index 760ecc7151..07df14e744 100644 --- a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java +++ b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java @@ -104,6 +104,7 @@ import com.raytheon.uf.edex.datadelivery.retrieval.RetrievalManagerNotifyEvent; * Feb 14, 2013 1596 djohnson Add test duplicating errors deleting multiple subscriptions for the same provider/dataset. * Mar 11, 2013 1645 djohnson Test configuration file modifications. * Mar 28, 2013 1841 djohnson Subscription is now UserSubscription. + * Apr 29, 2013 1910 djohnson Always shutdown bandwidth managers in tests. * * * @@ -418,8 +419,10 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { public void expiredSubscriptionUpdatedToNonExpiredIsScheduled() throws Exception { - final Date yesterday = new Date(TimeUtil.currentTimeMillis() - TimeUtil.MILLIS_PER_DAY); - final Date oneHourAgo = new Date(TimeUtil.currentTimeMillis() - TimeUtil.MILLIS_PER_HOUR); + final Date yesterday = new Date(TimeUtil.currentTimeMillis() + - TimeUtil.MILLIS_PER_DAY); + final Date oneHourAgo = new Date(TimeUtil.currentTimeMillis() + - TimeUtil.MILLIS_PER_HOUR); Subscription subscription = createSubscriptionThatFillsHalfABucket(); subscription.setSubscriptionStart(yesterday); @@ -428,7 +431,8 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { bandwidthManager.subscriptionUpdated(subscription); // Make sure nothing is scheduled when the subscription is expired - assertThat(bandwidthDao.getBandwidthAllocations(subscription.getRoute()), + assertThat( + bandwidthDao.getBandwidthAllocations(subscription.getRoute()), is(empty())); // No longer expired @@ -653,77 +657,89 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { subscription.getTime().setCycleTimes(Arrays.asList(Integer.valueOf(0))); bandwidthManager.schedule(subscription); - final BandwidthManager proposed = bandwidthManager - .startProposedBandwidthManager(BandwidthMap - .load(EdexBandwidthContextFactory - .getBandwidthMapConfig())); + BandwidthManager bwProposed = null; + try { + bwProposed = bandwidthManager + .startProposedBandwidthManager(BandwidthMap + .load(EdexBandwidthContextFactory + .getBandwidthMapConfig())); + final BandwidthManager proposed = bwProposed; - final BlockingQueue queue = new ArrayBlockingQueue( - 1); + final BlockingQueue queue = new ArrayBlockingQueue( + 1); - final int invocationCount = 10; - final CountDownLatch waitForAllThreadsReadyLatch = new CountDownLatch( - invocationCount * 2); - final CountDownLatch doneLatch = new CountDownLatch(invocationCount * 2); - for (int i = 0; i < invocationCount; i++) { - final int current = i; - Thread thread = new Thread() { - @Override - public void run() { - try { - // Wait for all threads to check in, then they all start - // working at once - waitForAllThreadsReadyLatch.countDown(); - waitForAllThreadsReadyLatch.await(); - proposed.updateDataSetMetaData(OpenDapGriddedDataSetMetaDataFixture.INSTANCE - .get(current)); - } catch (Exception e) { - queue.offer(e); + final int invocationCount = 10; + final CountDownLatch waitForAllThreadsReadyLatch = new CountDownLatch( + invocationCount * 2); + final CountDownLatch doneLatch = new CountDownLatch( + invocationCount * 2); + for (int i = 0; i < invocationCount; i++) { + final int current = i; + Thread thread = new Thread() { + @Override + public void run() { + try { + // Wait for all threads to check in, then they all + // start + // working at once + waitForAllThreadsReadyLatch.countDown(); + waitForAllThreadsReadyLatch.await(); + proposed.updateDataSetMetaData(OpenDapGriddedDataSetMetaDataFixture.INSTANCE + .get(current)); + } catch (Exception e) { + queue.offer(e); + } + doneLatch.countDown(); } - doneLatch.countDown(); - } - }; - thread.start(); - } + }; + thread.start(); + } - for (int i = 0; i < invocationCount; i++) { - final int current = i; - Thread thread = new Thread() { - @Override - public void run() { - try { - final Subscription subscription2 = SubscriptionFixture.INSTANCE - .get(current); - subscription2.addParameter(ParameterFixture.INSTANCE - .get(1)); - subscription2.addParameter(ParameterFixture.INSTANCE - .get(2)); - subscription2.addParameter(ParameterFixture.INSTANCE - .get(3)); - subscription2.getTime().setCycleTimes( - Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, - 11, 12, 13, 14, 15, 16, 17)); - subscription2.setLatencyInMinutes(current); - // Wait for all threads to check in, then they all start - // working at once - waitForAllThreadsReadyLatch.countDown(); - waitForAllThreadsReadyLatch.await(); - proposed.schedule(subscription2); - } catch (Exception e) { - queue.offer(e); + for (int i = 0; i < invocationCount; i++) { + final int current = i; + Thread thread = new Thread() { + @Override + public void run() { + try { + final Subscription subscription2 = SubscriptionFixture.INSTANCE + .get(current); + subscription2 + .addParameter(ParameterFixture.INSTANCE + .get(1)); + subscription2 + .addParameter(ParameterFixture.INSTANCE + .get(2)); + subscription2 + .addParameter(ParameterFixture.INSTANCE + .get(3)); + subscription2.getTime().setCycleTimes( + Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17)); + subscription2.setLatencyInMinutes(current); + // Wait for all threads to check in, then they all + // start + // working at once + waitForAllThreadsReadyLatch.countDown(); + waitForAllThreadsReadyLatch.await(); + proposed.schedule(subscription2); + } catch (Exception e) { + queue.offer(e); + } + doneLatch.countDown(); } - doneLatch.countDown(); - } - }; - thread.start(); - } + }; + thread.start(); + } - // Wait for all threads to finish - doneLatch.await(); + // Wait for all threads to finish + doneLatch.await(); - final Exception exception = queue.poll(); - if (exception != null) { - throw exception; + final Exception exception = queue.poll(); + if (exception != null) { + throw exception; + } + } finally { + shutdownBandwidthManager(bwProposed); } } diff --git a/tests/unit/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java b/tests/integration/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java similarity index 99% rename from tests/unit/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java rename to tests/integration/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java index afc0228437..5556e8496b 100644 --- a/tests/unit/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java +++ b/tests/integration/com/raytheon/uf/edex/registry/ebxml/services/validator/ValidatorImplTest.java @@ -64,6 +64,7 @@ import com.raytheon.uf.edex.registry.ebxml.util.EbxmlObjectUtil; * Date Ticket# Engineer Description * ------------ ---------- ----------- -------------------------- * Apr 23, 2013 1910 djohnson Initial creation + * Apr 29, 2013 1910 djohnson Move to integration tests section. * * * diff --git a/tests/unit/com/raytheon/uf/edex/datadelivery/retrieval/handlers/RetrievalTaskTest.java b/tests/unit/com/raytheon/uf/edex/datadelivery/retrieval/handlers/RetrievalTaskTest.java index 423515279b..393e07d788 100644 --- a/tests/unit/com/raytheon/uf/edex/datadelivery/retrieval/handlers/RetrievalTaskTest.java +++ b/tests/unit/com/raytheon/uf/edex/datadelivery/retrieval/handlers/RetrievalTaskTest.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentLinkedQueue; +import org.junit.After; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -84,6 +85,7 @@ import com.raytheon.uf.edex.datadelivery.retrieval.interfaces.IRetrievalResponse * Feb 15, 2013 1543 djohnson Class renames. * Mar 05, 2013 1647 djohnson Pass wmo header strategy to constructor. * Mar 19, 2013 1794 djohnson RetrievalTasks integrate at a queue. + * Apr 29, 2013 1910 djohnson Unregister from EventBus after each test. * * * @@ -165,6 +167,11 @@ public class RetrievalTaskTest { EventBus.register(this); } + @After + public void tearDown() { + EventBus.unregister(this); + } + @Test public void processesRetrievalForItsSpecifiedNetwork() throws DataAccessLayerException {