You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ma...@apache.org on 2016/04/20 16:29:47 UTC
[2/4] activemq-artemis git commit: Protected ActiveMQClient API
against misuse.
Protected ActiveMQClient API against misuse.
1. Changed public fields in ActiveMQClient to private and added getters.
Exposing fields for thread pool sized allow to modify them in undesired ways.
I made these fields private and added corresponding getter methods.
In addition, I renamed the field 'globalThreadMaxPoolSize'
to 'globalThreadPoolSize' to be more consistent with the
'globalScheduledThreadPoolSize' field name.
I also adapted some tests to always call clearThreadPools after
the thread pool size configuration has been changed.
2. Protect against injecting null as thread pools
ActiveMQClient.injectPools allowed null as injected thread pools.
The effect was that internal threads pools were created,
but not shutdown correctly.
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/1b5396c0
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/1b5396c0
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/1b5396c0
Branch: refs/heads/master
Commit: 1b5396c033774b85eefce150e906117928cda75b
Parents: 2360fb4
Author: Bernd Gutjahr <be...@hpe.com>
Authored: Fri Apr 15 09:14:30 2016 +0200
Committer: Martyn Taylor <mt...@redhat.com>
Committed: Wed Apr 20 15:29:25 2016 +0100
----------------------------------------------------------------------
.../artemis/api/core/client/ActiveMQClient.java | 23 +++++++++++++-------
.../activemq/artemis/ClientThreadPoolsTest.java | 8 +++++--
.../core/remoting/impl/invm/InVMConnector.java | 4 ++--
.../integration/client/CoreClientTest.java | 5 +++--
4 files changed, 26 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/1b5396c0/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ActiveMQClient.java
----------------------------------------------------------------------
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ActiveMQClient.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ActiveMQClient.java
index 1674a53..abc3a5d 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ActiveMQClient.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ActiveMQClient.java
@@ -46,9 +46,9 @@ import org.apache.activemq.artemis.utils.ActiveMQThreadFactory;
*/
public final class ActiveMQClient {
- public static int globalThreadMaxPoolSize;
+ private static int globalThreadPoolSize;
- public static int globalScheduledThreadPoolSize;
+ private static int globalScheduledThreadPoolSize;
public static final String DEFAULT_CONNECTION_LOAD_BALANCING_POLICY_CLASS_NAME = RoundRobinConnectionLoadBalancingPolicy.class.getCanonicalName();
@@ -195,13 +195,15 @@ public final class ActiveMQClient {
}
/** Warning: This method has to be called before any clients or servers is started on the JVM otherwise previous ServerLocator would be broken after this call. */
- public static synchronized void injectPools(ThreadPoolExecutor globalThreadPool, ScheduledThreadPoolExecutor scheduledThreadPoolExecutor) {
+ public static synchronized void injectPools(ThreadPoolExecutor globalThreadPool, ScheduledThreadPoolExecutor scheduledThreadPool) {
+ if (globalThreadPool == null || scheduledThreadPool == null)
+ throw new IllegalArgumentException("thread pools must not be null");
// We call clearThreadPools as that will shutdown any previously used executor
clearThreadPools();
ActiveMQClient.globalThreadPool = globalThreadPool;
- ActiveMQClient.globalScheduledThreadPool = scheduledThreadPoolExecutor;
+ ActiveMQClient.globalScheduledThreadPool = scheduledThreadPool;
injectedPools = true;
}
@@ -214,11 +216,11 @@ public final class ActiveMQClient {
}
});
- if (globalThreadMaxPoolSize == -1) {
+ if (globalThreadPoolSize == -1) {
globalThreadPool = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<Runnable>(), factory);
}
else {
- globalThreadPool = new ThreadPoolExecutor(ActiveMQClient.globalThreadMaxPoolSize, ActiveMQClient.globalThreadMaxPoolSize, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(), factory);
+ globalThreadPool = new ThreadPoolExecutor(ActiveMQClient.globalThreadPoolSize, ActiveMQClient.globalThreadPoolSize, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(), factory);
}
}
return globalThreadPool;
@@ -238,8 +240,13 @@ public final class ActiveMQClient {
return globalScheduledThreadPool;
}
+ public static int getGlobalThreadPoolSize() {
+ return globalThreadPoolSize;
+ }
-
+ public static int getGlobalScheduledThreadPoolSize() {
+ return globalScheduledThreadPoolSize;
+ }
/**
* Initializes the global thread pools properties from System properties. This method will update the global
@@ -273,7 +280,7 @@ public final class ActiveMQClient {
if (globalThreadMaxPoolSize < 2 && globalThreadMaxPoolSize != -1) globalThreadMaxPoolSize = 2;
ActiveMQClient.globalScheduledThreadPoolSize = globalScheduledThreadPoolSize;
- ActiveMQClient.globalThreadMaxPoolSize = globalThreadMaxPoolSize;
+ ActiveMQClient.globalThreadPoolSize = globalThreadMaxPoolSize;
}
/**
http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/1b5396c0/artemis-core-client/src/test/java/org/apache/activemq/artemis/ClientThreadPoolsTest.java
----------------------------------------------------------------------
diff --git a/artemis-core-client/src/test/java/org/apache/activemq/artemis/ClientThreadPoolsTest.java b/artemis-core-client/src/test/java/org/apache/activemq/artemis/ClientThreadPoolsTest.java
index 57399cd..a3dc213 100644
--- a/artemis-core-client/src/test/java/org/apache/activemq/artemis/ClientThreadPoolsTest.java
+++ b/artemis-core-client/src/test/java/org/apache/activemq/artemis/ClientThreadPoolsTest.java
@@ -54,7 +54,7 @@ public class ClientThreadPoolsTest {
System.clearProperty(ActiveMQClient.SCHEDULED_THREAD_POOL_SIZE_PROPERTY_KEY);
ActiveMQClient.initializeGlobalThreadPoolProperties();
ActiveMQClient.clearThreadPools();
- Assert.assertEquals(ActiveMQClient.DEFAULT_GLOBAL_THREAD_POOL_MAX_SIZE, ActiveMQClient.globalThreadMaxPoolSize);
+ Assert.assertEquals(ActiveMQClient.DEFAULT_GLOBAL_THREAD_POOL_MAX_SIZE, ActiveMQClient.getGlobalThreadPoolSize());
}
@Test
@@ -65,14 +65,15 @@ public class ClientThreadPoolsTest {
System.setProperty(ActiveMQClient.THREAD_POOL_MAX_SIZE_PROPERTY_KEY, "" + threadPoolMaxSize);
System.setProperty(ActiveMQClient.SCHEDULED_THREAD_POOL_SIZE_PROPERTY_KEY, "" + scheduledThreadPoolSize);
ActiveMQClient.initializeGlobalThreadPoolProperties();
+ ActiveMQClient.clearThreadPools();
testSystemPropertiesThreadPoolSettings(threadPoolMaxSize, scheduledThreadPoolSize);
}
@Test
public void testShutdownPoolInUse() throws Exception {
- ActiveMQClient.clearThreadPools();
ActiveMQClient.setGlobalThreadPoolProperties(10, 1);
+ ActiveMQClient.clearThreadPools();
final CountDownLatch inUse = new CountDownLatch(1);
final CountDownLatch neverLeave = new CountDownLatch(1);
@@ -146,6 +147,7 @@ public class ClientThreadPoolsTest {
int testScheduleSize = 9;
ActiveMQClient.setGlobalThreadPoolProperties(testMaxSize, testScheduleSize);
+ ActiveMQClient.clearThreadPools();
testSystemPropertiesThreadPoolSettings(testMaxSize, testScheduleSize);
}
@@ -156,6 +158,7 @@ public class ClientThreadPoolsTest {
int testScheduleSize = 9;
ActiveMQClient.setGlobalThreadPoolProperties(testMaxSize, testScheduleSize);
+ ActiveMQClient.clearThreadPools();
testSystemPropertiesThreadPoolSettings(testMaxSize, testScheduleSize);
}
@@ -255,6 +258,7 @@ public class ClientThreadPoolsTest {
// Resets the global thread pool properties back to default.
System.setProperties(systemProperties);
ActiveMQClient.initializeGlobalThreadPoolProperties();
+ ActiveMQClient.clearThreadPools();
}
http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/1b5396c0/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnector.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnector.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnector.java
index 9f965ea..d39b8c6 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnector.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnector.java
@@ -103,11 +103,11 @@ public class InVMConnector extends AbstractConnector {
private static synchronized ExecutorService getInVMExecutor() {
if (threadPoolExecutor == null) {
- if (ActiveMQClient.globalThreadMaxPoolSize <= -1) {
+ if (ActiveMQClient.getGlobalThreadPoolSize() <= -1) {
threadPoolExecutor = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<Runnable>(), Executors.defaultThreadFactory());
}
else {
- threadPoolExecutor = Executors.newFixedThreadPool(ActiveMQClient.globalThreadMaxPoolSize);
+ threadPoolExecutor = Executors.newFixedThreadPool(ActiveMQClient.getGlobalThreadPoolSize());
}
}
return threadPoolExecutor;
http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/1b5396c0/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/CoreClientTest.java
----------------------------------------------------------------------
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/CoreClientTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/CoreClientTest.java
index a4ef71a..c9b8944 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/CoreClientTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/CoreClientTest.java
@@ -74,11 +74,12 @@ public class CoreClientTest extends ActiveMQTestBase {
@Test
public void testCoreClientWithGlobalThreadPoolParamtersChanged() throws Exception {
- int originalScheduled = ActiveMQClient.globalScheduledThreadPoolSize;
- int originalGlobal = ActiveMQClient.globalThreadMaxPoolSize;
+ int originalScheduled = ActiveMQClient.getGlobalScheduledThreadPoolSize();
+ int originalGlobal = ActiveMQClient.getGlobalThreadPoolSize();
try {
ActiveMQClient.setGlobalThreadPoolProperties(2, 1);
+ ActiveMQClient.clearThreadPools();
ServerLocator locator = createNonHALocator(false);
testCoreClient(true, locator);
}