Hi I just spotted bunch of flaws in that utility class and logged a ticket for it:
https://issues.apache.org/jira/browse/CAMEL-6951 Babak Claus Ibsen-2 wrote > Hi > > There is a stopAndShutdownService method on ServiceHelper in the util > package. Its maybe nicer to use to than the type cast. Then you can do > that in one command. > > And this is also how we stop/start/shutdown services in a cleaner way. > And if you enabled TRACE/DEBUG logging the service helper can log what > happens as well. > > On Fri, Nov 8, 2013 at 9:46 PM, < > bvahdat@ > > wrote: >> Updated Branches: >> refs/heads/master f74ff2cf6 -> f744afd9c >> >> >> CAMEL-6948: Releasing a non-singleton producer by ProducerCache should >> not only stop the producer but also shutdown it as well if it is a >> ShutdownableService. >> >> Project: http://git-wip-us.apache.org/repos/asf/camel/repo >> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/f744afd9 >> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/f744afd9 >> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/f744afd9 >> >> Branch: refs/heads/master >> Commit: f744afd9c39a1fc9261ad68dabe06a51a238fac9 >> Parents: f74ff2c >> Author: Babak Vahdat < > bvahdat@ > > >> Authored: Fri Nov 8 21:45:59 2013 +0100 >> Committer: Babak Vahdat < > bvahdat@ > > >> Committed: Fri Nov 8 21:45:59 2013 +0100 >> >> ---------------------------------------------------------------------- >> .../org/apache/camel/impl/ProducerCache.java | 6 +++ >> .../camel/impl/DefaultProducerCacheTest.java | 53 >> +++++++++++++++++--- >> 2 files changed, 51 insertions(+), 8 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java >> ---------------------------------------------------------------------- >> diff --git >> a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java >> b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java >> index 6b292c0..b35eca5 100644 >> --- a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java >> +++ b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java >> @@ -30,6 +30,7 @@ import org.apache.camel.Processor; >> import org.apache.camel.Producer; >> import org.apache.camel.ProducerCallback; >> import org.apache.camel.ServicePoolAware; >> +import org.apache.camel.ShutdownableService; >> import org.apache.camel.processor.UnitOfWorkProducer; >> import org.apache.camel.spi.ServicePool; >> import org.apache.camel.support.ServiceSupport; >> @@ -137,6 +138,11 @@ public class ProducerCache extends ServiceSupport { >> } else if (!producer.isSingleton()) { >> // stop non singleton producers as we should not leak >> resources >> producer.stop(); >> + >> + // shutdown as well in case the producer is shutdownable >> + if (producer instanceof ShutdownableService) { >> + ShutdownableService.class.cast(producer).shutdown(); >> + } >> } >> } >> >> >> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java >> ---------------------------------------------------------------------- >> diff --git >> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java >> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java >> index 5ad0f42..803302c 100644 >> --- >> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java >> +++ >> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java >> @@ -30,7 +30,8 @@ import org.apache.camel.Producer; >> */ >> public class DefaultProducerCacheTest extends ContextTestSupport { >> >> - private static final AtomicInteger COUNTER = new AtomicInteger(); >> + private final AtomicInteger stopCounter = new AtomicInteger(); >> + private final AtomicInteger shutdownCounter = new AtomicInteger(); >> >> public void testCacheProducerAcquireAndRelease() throws Exception { >> ProducerCache cache = new ProducerCache(this, context); >> @@ -56,7 +57,7 @@ public class DefaultProducerCacheTest extends >> ContextTestSupport { >> assertEquals("Size should be 0", 0, cache.size()); >> >> for (int i = 0; i < 8; i++) { >> - Endpoint e = new MyEndpoint(i); >> + Endpoint e = new MyEndpoint(true, i); >> Producer p = cache.acquireProducer(e); >> cache.releaseProducer(e, p); >> } >> @@ -64,19 +65,50 @@ public class DefaultProducerCacheTest extends >> ContextTestSupport { >> assertEquals("Size should be 5", 5, cache.size()); >> >> // should have stopped the 3 evicted >> - assertEquals(3, COUNTER.get()); >> + assertEquals(3, stopCounter.get()); >> >> cache.stop(); >> >> // should have stopped all 8 >> - assertEquals(8, COUNTER.get()); >> + assertEquals(8, stopCounter.get()); >> + } >> + >> + public void >> testReleaseProducerInvokesStopAndShutdownByNonSingeltonProducers() throws >> Exception { >> + ProducerCache cache = new ProducerCache(this, context, 1); >> + cache.start(); >> + >> + assertEquals("Size should be 0", 0, cache.size()); >> + >> + for (int i = 0; i < 3; i++) { >> + Endpoint e = new MyEndpoint(false, i); >> + Producer p = cache.acquireProducer(e); >> + cache.releaseProducer(e, p); >> + } >> + >> + assertEquals("Size should be 0", 0, cache.size()); >> + >> + // should have stopped all 3 >> + assertEquals(3, stopCounter.get()); >> + >> + // should have shutdown all 3 >> + assertEquals(3, shutdownCounter.get()); >> + >> + cache.stop(); >> + >> + // no more stop after stopping the cache >> + assertEquals(3, stopCounter.get()); >> + >> + // no more shutdown after stopping the cache >> + assertEquals(3, shutdownCounter.get()); >> } >> >> private final class MyEndpoint extends DefaultEndpoint { >> >> - private int number; >> + private final boolean isSingleton; >> + private final int number; >> >> - private MyEndpoint(int number) { >> + private MyEndpoint(boolean isSingleton, int number) { >> + this.isSingleton = isSingleton; >> this.number = number; >> } >> >> @@ -92,7 +124,7 @@ public class DefaultProducerCacheTest extends >> ContextTestSupport { >> >> @Override >> public boolean isSingleton() { >> - return true; >> + return isSingleton; >> } >> >> @Override >> @@ -114,7 +146,12 @@ public class DefaultProducerCacheTest extends >> ContextTestSupport { >> >> @Override >> protected void doStop() throws Exception { >> - COUNTER.incrementAndGet(); >> + stopCounter.incrementAndGet(); >> + } >> + >> + @Override >> + protected void doShutdown() throws Exception { >> + shutdownCounter.incrementAndGet(); >> } >> } >> >> > > > > -- > Claus Ibsen > ----------------- > Red Hat, Inc. > Email: > cibsen@ > Twitter: davsclaus > Blog: http://davsclaus.com > Author of Camel in Action: http://www.manning.com/ibsen -- View this message in context: http://camel.465427.n5.nabble.com/Re-git-commit-CAMEL-6948-Releasing-a-non-singleton-producer-by-ProducerCache-should-not-only-stop-th-tp5742926p5742957.html Sent from the Camel Development mailing list archive at Nabble.com.
