From 40e042391c60329002e4d8db49dcee3a11f11de2 Mon Sep 17 00:00:00 2001 From: Dylan Schell Date: Mon, 28 Sep 2015 09:05:17 +0200 Subject: [PATCH] * fix thread leak by changing the fixed threadpool into a caching thread pool. * alter thread factory to provide custom names for the threads, so they are easier to identify in thread dumps. --- .../LoadingMessageSourceProvider.java | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/github/fge/msgsimple/provider/LoadingMessageSourceProvider.java b/src/main/java/com/github/fge/msgsimple/provider/LoadingMessageSourceProvider.java index 7eff6df..81875f5 100644 --- a/src/main/java/com/github/fge/msgsimple/provider/LoadingMessageSourceProvider.java +++ b/src/main/java/com/github/fge/msgsimple/provider/LoadingMessageSourceProvider.java @@ -36,10 +36,13 @@ import java.util.concurrent.Executors; import java.util.concurrent.FutureTask; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; /** * A caching, on-demand loading message source provider with configurable expiry @@ -75,24 +78,33 @@ public final class LoadingMessageSourceProvider implements MessageSourceProvider { - /* - * Use daemon threads. We don't give control to the user about the - * ExecutorService, and we don't have a reliable way to shut it down (a JVM - * shutdown hook does not get involved on a webapp shutdown, so we cannot - * use that...). - */ - private static final ThreadFactory THREAD_FACTORY = new ThreadFactory() - { - private final ThreadFactory factory = Executors.defaultThreadFactory(); + public static class MsgSimpleThreadFactory implements ThreadFactory { + private static final AtomicInteger poolNumber = new AtomicInteger(1); + private final ThreadGroup group; + private final AtomicInteger threadNumber = new AtomicInteger(1); + private final String namePrefix; - @Override - public Thread newThread(final Runnable r) - { - final Thread ret = factory.newThread(r); - ret.setDaemon(true); - return ret; + MsgSimpleThreadFactory() { + SecurityManager s = System.getSecurityManager(); + group = (s != null) ? s.getThreadGroup() : + Thread.currentThread().getThreadGroup(); + namePrefix = "msg-simple-pool-" + + poolNumber.getAndIncrement() + + "-thread-"; } - }; + + public Thread newThread(Runnable r) { + Thread t = new Thread(group, r, + namePrefix + threadNumber.getAndIncrement(), + 0); + t.setDaemon(true); + if (t.getPriority() != Thread.NORM_PRIORITY) + t.setPriority(Thread.NORM_PRIORITY); + return t; + } + } + + private static final ThreadFactory THREAD_FACTORY = new MsgSimpleThreadFactory(); private static final InternalBundle BUNDLE = InternalBundle.getInstance(); @@ -101,8 +113,10 @@ public Thread newThread(final Runnable r) /* * Executor service for loading tasks */ - private final ExecutorService service - = Executors.newFixedThreadPool(NTHREADS, THREAD_FACTORY); + private final ExecutorService service = new ThreadPoolExecutor(0, NTHREADS, + 60L,TimeUnit.SECONDS, + new SynchronousQueue(), + THREAD_FACTORY); /* * Loader and default source @@ -394,4 +408,4 @@ public MessageSourceProvider build() return new LoadingMessageSourceProvider(this); } } -} +} \ No newline at end of file