Hi Babak, Thanks for the book reference, it made a good point, I will update the code with your suggestion shortly.
-- Willem Jiang Red Hat, Inc. Web: http://www.redhat.com Blog: http://willemjiang.blogspot.com (English) http://jnn.iteye.com (Chinese) Twitter: willemjiang Weibo: 姜宁willem On December 1, 2014 at 10:23:17 PM, Babak Vahdat (babak.vah...@swissonline.ch) wrote: > > > Am 01.12.14 14:49 schrieb "Willem Jiang" unter : > > >Hi Babak, > > > >As we just create the UnstoppableTime instance inside of the > >NettyClientBossPoolBuilder and don’t want to other people to create the > >instance out side of NettyClientBossPoolBuilder, I don’t think we need to > >define the UnstoppableTime as the static member class. > > Maybe I was not clear enough about what I mean: > > Instead of the current class declaration which is: > class UnstoppableTimer implements Timer { > > We should better do: > private static class UnstoppableTimer implements Timer { > > That is: > > add 'private' => Instead of the current 'package private' access we get a > more restricted visibility which would be *preciser*. > add 'static' => avoid the extraneous reference to its *every* enclosing > instance > > See also Effective Java, Item 22, page 106: Favor static member classes > over nonstatic > > Babak > > > > > >-- > >Willem Jiang > > > >Red Hat, Inc. > >Web: http://www.redhat.com > >Blog: http://willemjiang.blogspot.com (English) > >http://jnn.iteye.com (Chinese) > >Twitter: willemjiang > >Weibo: 姜宁willem > > > > > > > >On December 1, 2014 at 1:57:31 PM, Babak Vahdat > >(babak.vah...@swissonline.ch) wrote: > >> Hi Willem, > >> > >> Yeah that makes sense. > >> > >> I guess the newly introduced UnstoppableTimer should be better declared > >>as > >> (private) *static* member class instead of nonstatic. This would avoid > >>the > >> extraneous reference to its *every* enclosing instance (which is > >> NettyClientBossPoolBuilder). > >> > >> Babak > >> > >> > >> Am 01.12.14 05:57 schrieb "Willem Jiang" unter : > >> > >> >Hi Babak, > >> > > >> >As the we share the timer across the camel producers, we cannot just > >> >simply call the releaseExternalResources method which stops the shared > >> >timer here. > >> > > >> >So I did change on the NettyClientBossPoolBuilder to avoid stopping the > >> >timer by wrapping it > >> > > >> >-- > >> >Willem Jiang > >> > > >> >Red Hat, Inc. > >> >Web: http://www.redhat.com > >> >Blog: http://willemjiang.blogspot.com (English) > >> >http://jnn.iteye.com (Chinese) > >> >Twitter: willemjiang > >> >Weibo: 姜宁willem > >> > > >> > > >> > > >> >On November 30, 2014 at 7:06:57 PM, bvah...@apache.org > >> >(bvah...@apache.org) wrote: > >> >> Repository: camel > >> >> Updated Branches: > >> >> refs/heads/master 2f4010e03 -> 829458c70 > >> >> > >> >> > >> >> CAMEL-8094: Do not use org.jboss.netty.util.internal.ExecutorUtil as > >>it > >> >>breaks > >> >> the camel-netty Karaf feature > >> >> > >> >> Project: http://git-wip-us.apache.org/repos/asf/camel/repo > >> >> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/829458c7 > >> >> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/829458c7 > >> >> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/829458c7 > >> >> > >> >> Branch: refs/heads/master > >> >> Commit: 829458c700a8dcbaa0d710a7d77ff12636afa0a7 > >> >> Parents: 2f4010e > >> >> Author: Babak Vahdat > >> >> Authored: Sun Nov 30 12:06:09 2014 +0100 > >> >> Committer: Babak Vahdat > >> >> Committed: Sun Nov 30 12:06:09 2014 +0100 > >> >> > >> >> > >>---------------------------------------------------------------------- > >> >> .../netty/NettyClientBossPoolBuilder.java | 16 ----------- > >> >> .../camel/component/netty/NettyProducer.java | 14 +++++++--- > >> >> .../netty/NettyServerBossPoolBuilder.java | 18 +------------ > >> >> .../component/netty/NettyWorkerPoolBuilder.java | 28 > >> >>+------------------- > >> >> 4 files changed, 13 insertions(+), 63 deletions(-) > >> >> > >>---------------------------------------------------------------------- > >> >> > >> >> > >> >> > >> > >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c > >>>>am > >> > >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyClientBoss > >>>>Po > >> >>olBuilder.java > >> >> > >>---------------------------------------------------------------------- > >> >> diff --git > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyClientBossPoolBuilder.java > >> >> > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyClientBossPoolBuilder.java > >> >> index cc9bfb0..62caf1e 100644 > >> >> --- > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyClientBossPoolBuilder.java > >> >> +++ > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyClientBossPoolBuilder.java > >> >> @@ -23,7 +23,6 @@ import org.jboss.netty.channel.socket.nio.BossPool; > >> >> import org.jboss.netty.channel.socket.nio.NioClientBossPool; > >> >> import org.jboss.netty.util.ThreadNameDeterminer; > >> >> import org.jboss.netty.util.Timer; > >> >> -import org.jboss.netty.util.internal.ExecutorUtil; > >> >> > >> >> /** > >> >> * A builder to create Netty {@link > >> >>org.jboss.netty.channel.socket.nio.BossPool} > >> >> which can be used for sharing boss pools > >> >> @@ -78,19 +77,4 @@ public final class NettyClientBossPoolBuilder { > >> >> BossPool build() { > >> >> return new NioClientBossPool(Executors.newCachedThreadPool(), > >> >>bossCount, timer, > >> >> new CamelNettyThreadNameDeterminer(pattern, name)); > >> >> } > >> >> - > >> >> - class CamelNioClientBossPool extends NioClientBossPool { > >> >> - private Executor executor; > >> >> - CamelNioClientBossPool(Executor bossExecutor, int bossCount, Timer > >> >>timer, ThreadNameDeterminer > >> >> determiner) { > >> >> - super(bossExecutor, bossCount, timer, determiner); > >> >> - executor = bossExecutor; > >> >> - } > >> >> - > >> >> - // Just make sure we shutdown the executor; > >> >> - public void shutdown() { > >> >> - super.shutdown(); > >> >> - ExecutorUtil.shutdownNow(executor); > >> >> - } > >> >> - > >> >> - } > >> >> } > >> >> > >> >> > >> > >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c > >>>>am > >> > >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyProducer.j > >>>>av > >> >>a > >> >> > >>---------------------------------------------------------------------- > >> >> diff --git > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyProducer.java > >> >> > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyProducer.java > >> >> index 491ee38..ac1ecef 100644 > >> >> --- > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyProducer.java > >> >> +++ > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyProducer.java > >> >> @@ -52,6 +52,7 @@ import > >> >>org.jboss.netty.channel.socket.nio.NioClientSocketChannelFactory; > >> >> import org.jboss.netty.channel.socket.nio.NioDatagramChannelFactory; > >> >> import org.jboss.netty.channel.socket.nio.NioDatagramWorkerPool; > >> >> import org.jboss.netty.channel.socket.nio.WorkerPool; > >> >> +import org.jboss.netty.util.ExternalResourceReleasable; > >> >> import org.slf4j.Logger; > >> >> import org.slf4j.LoggerFactory; > >> >> > >> >> @@ -161,7 +162,12 @@ public class NettyProducer extends > >> >>DefaultAsyncProducer { > >> >> bossPool = null; > >> >> } > >> >> if (workerPool != null) { > >> >> - workerPool.shutdown(); > >> >> + if (workerPool instanceof ExternalResourceReleasable) { > >> >> + // this will first invoke workerPool#shutdown() internally (e.g. > >> >>org.jboss.netty.channel.socket.nio.AbstractNioWorkerPool) > >> >> + ((ExternalResourceReleasable) > >>workerPool).releaseExternalResources(); > >> >> + } else { > >> >> + workerPool.shutdown(); > >> >> + } > >> >> workerPool = null; > >> >> } > >> >> > >> >> @@ -174,12 +180,14 @@ public class NettyProducer extends > >> >>DefaultAsyncProducer { > >> >> } > >> >> > >> >> if (channelFactory != null) { > >> >> - channelFactory.shutdown(); > >> >> + // this will first invoke channelFactory#shutdown() internally (see > >> >>it's javadoc) > >> >> + channelFactory.releaseExternalResources(); > >> >> channelFactory = null; > >> >> } > >> >> > >> >> if (datagramChannelFactory != null) { > >> >> - datagramChannelFactory.shutdown(); > >> >> + // this will first invoke datagramChannelFactory#shutdown() > >> >>internally (see it's > >> >> javadoc) > >> >> + datagramChannelFactory.releaseExternalResources(); > >> >> datagramChannelFactory = null; > >> >> } > >> >> > >> >> > >> >> > >> > >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c > >>>>am > >> > >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyServerBoss > >>>>Po > >> >>olBuilder.java > >> >> > >>---------------------------------------------------------------------- > >> >> diff --git > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyServerBossPoolBuilder.java > >> >> > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyServerBossPoolBuilder.java > >> >> index 3be5a64..6404ae1 100644 > >> >> --- > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyServerBossPoolBuilder.java > >> >> +++ > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyServerBossPoolBuilder.java > >> >> @@ -22,7 +22,6 @@ import java.util.concurrent.Executors; > >> >> import org.jboss.netty.channel.socket.nio.BossPool; > >> >> import org.jboss.netty.channel.socket.nio.NioServerBossPool; > >> >> import org.jboss.netty.util.ThreadNameDeterminer; > >> >> -import org.jboss.netty.util.internal.ExecutorUtil; > >> >> > >> >> /** > >> >> * A builder to create Netty {@link > >> >>org.jboss.netty.channel.socket.nio.BossPool} > >> >> which can be used for sharing boss pools > >> >> @@ -65,21 +64,6 @@ public final class NettyServerBossPoolBuilder { > >> >> * Creates a new boss pool. > >> >> */ > >> >> BossPool build() { > >> >> - return new CamelNioServerBossPool(Executors.newCachedThreadPool(), > >> >>bossCount, > >> >> new CamelNettyThreadNameDeterminer(pattern, name)); > >> >> - } > >> >> - > >> >> - class CamelNioServerBossPool extends NioServerBossPool { > >> >> - private Executor executor; > >> >> - CamelNioServerBossPool(Executor bossExecutor, int bossCount, > >> >>ThreadNameDeterminer > >> >> determiner) { > >> >> - super(bossExecutor, bossCount, determiner); > >> >> - executor = bossExecutor; > >> >> - } > >> >> - > >> >> - // Just make sure we shutdown the executor; > >> >> - public void shutdown() { > >> >> - super.shutdown(); > >> >> - ExecutorUtil.shutdownNow(executor); > >> >> - } > >> >> - > >> >> + return new NioServerBossPool(Executors.newCachedThreadPool(), > >> >>bossCount, new > >> >> CamelNettyThreadNameDeterminer(pattern, name)); > >> >> } > >> >> } > >> >> > >> >> > >> > >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c > >>>>am > >> > >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyWorkerPool > >>>>Bu > >> >>ilder.java > >> >> > >>---------------------------------------------------------------------- > >> >> diff --git > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyWorkerPoolBuilder.java > >> >> > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyWorkerPoolBuilder.java > >> >> index 5b6253a..c60cc25 100644 > >> >> --- > >> > >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyWorkerPoolBuilder.java > >> >> +++ > >> > >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty > >>>>/N > >> >>ettyWorkerPoolBuilder.java > >> >> @@ -22,7 +22,6 @@ import java.util.concurrent.Executors; > >> >> import org.jboss.netty.channel.socket.nio.NioWorkerPool; > >> >> import org.jboss.netty.channel.socket.nio.WorkerPool; > >> >> import org.jboss.netty.util.ThreadNameDeterminer; > >> >> -import org.jboss.netty.util.internal.ExecutorUtil; > >> >> > >> >> /** > >> >> * A builder to create Netty {@link WorkerPool} which can be used for > >> >>sharing worker pools > >> >> @@ -67,32 +66,7 @@ public final class NettyWorkerPoolBuilder { > >> >> */ > >> >> public WorkerPool build() { > >> >> int count = workerCount > 0 ? workerCount : > >> >>NettyHelper.DEFAULT_IO_THREADS; > >> >> - workerPool = new > >>CamelNioWorkerPool(Executors.newCachedThreadPool(), > >> >>count, > >> >> new CamelNettyThreadNameDeterminer(pattern, name)); > >> >> + workerPool = new NioWorkerPool(Executors.newCachedThreadPool(), > >> >>count, new CamelNettyThreadNameDeterminer(pattern, > >> >> name)); > >> >> return workerPool; > >> >> } > >> >> - > >> >> - class CamelNioWorkerPool extends NioWorkerPool { > >> >> - private Executor executor; > >> >> - CamelNioWorkerPool(Executor workerExecutor, int count, > >> >>ThreadNameDeterminer > >> >> determiner) { > >> >> - super(workerExecutor, count, determiner); > >> >> - executor = workerExecutor; > >> >> - } > >> >> - > >> >> - // Just make sure we shutdown the executor; > >> >> - public void shutdown() { > >> >> - super.shutdown(); > >> >> - ExecutorUtil.shutdownNow(executor); > >> >> - } > >> >> - > >> >> - } > >> >> - > >> >> - /** > >> >> - * Shutdown the created worker pool > >> >> - */ > >> >> - public void destroy() { > >> >> - if (workerPool != null) { > >> >> - workerPool.shutdown(); > >> >> - workerPool = null; > >> >> - } > >> >> - } > >> >> } > >> >> > >> >> > >> > > >> > >> > >> > > > > >