From 21c0be5e2a532478fe3f418de17bb07d30a8e394 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 27 Feb 2015 23:38:13 +0100 Subject: [PATCH] Fixed AbstractXMPPConnection.cachedExecutorService the combination with concurrencyLevel and LinkedBlockingQueue never worked as intented. The idea was that the cachedExecutorService would spawn new threads until maximumPoolSize (=concurrencyLevel) is reached, and then start queing the Runnables. But this was not the case, since ThreadPoolExecutor does not take into consideration if the worker threads is busy, i.e. executing a Runnable, or idle, i.e. waiting for a Runnable. This means that if a busy Worker would execute a Runnable, which would block, because it's waiting for an event (e.g. an incoming IQ request), then the handling of those incoming IQ request would be queued by ThreadPoolExecutor, because no fewer threads then corePoolSize are running and the task can be queued (since the LinkedBlockingQueue is unbounded). --- .../smack/AbstractXMPPConnection.java | 39 +------------------ 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index 35a64043e..bf47eaf0d 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -32,7 +32,6 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadPoolExecutor; @@ -247,46 +246,12 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { private final ScheduledExecutorService removeCallbacksService = Executors.newSingleThreadScheduledExecutor( new SmackExecutorThreadFactory(connectionCounterValue, "Remove Callbacks")); - private static int concurrencyLevel = Runtime.getRuntime().availableProcessors() + 1; - /** - * Set the concurrency level used by newly created connections. - *

- * The concurrency level determines the maximum pool size of the executor service that is used to e.g. invoke - * callbacks and IQ request handlers. - *

- *

- * The default value is Runtime.getRuntime().availableProcessors() + 1. Note that the number of - * available processors may change at runtime. So you may need to adjust it to your enviornment, although in most - * cases this should not be necessary. - *

- * - * @param concurrencyLevel the concurrency level used by new connections. - */ - public static void setConcurrencyLevel(int concurrencyLevel) { - if (concurrencyLevel < 1) { - throw new IllegalArgumentException("concurrencyLevel must be greater than zero"); - } - AbstractXMPPConnection.concurrencyLevel = concurrencyLevel; - } - - /** - * The constant long '120'. - */ - private static final long THREAD_KEEP_ALIVE_SECONDS = 60L * 2; - - /** - * Creates an executor service just as {@link Executors#newCachedThreadPool()} would do, but with a keep alive time - * of 2 minutes instead of 60 seconds. And a custom thread factory to set meaningful names on the threads and set + * A cached thread pool executor service with custom thread factory to set meaningful names on the threads and set * them 'daemon'. */ - private final ExecutorService cachedExecutorService = new ThreadPoolExecutor( + private final ExecutorService cachedExecutorService = Executors.newCachedThreadPool( // @formatter:off - 0, // corePoolSize - concurrencyLevel, // maximumPoolSize - THREAD_KEEP_ALIVE_SECONDS, // keepAliveTime - TimeUnit.SECONDS, // keepAliveTime unit, note that MINUTES is Android API 9 - new LinkedBlockingQueue(), // workQueue new SmackExecutorThreadFactory( // threadFactory connectionCounterValue, "Cached Executor"