From 7d6a6de8dde9700995771daddcf505b78cd58e45 Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Wed, 4 Nov 2015 11:38:43 +0100 Subject: [PATCH] [ARTEMIS-294] Use doPriviledged block for ServiceLoader In ServiceUtils, loads services from an AccessController.doPriviledged block and use the ServiceUtils's own classloader instead of the TCCL (that may be different depending on who's requesting a managed connection factory first) JIRA: https://issues.apache.org/jira/browse/ARTEMIS-294 --- .../service/extensions/ServiceUtils.java | 55 ++++++++++++++----- .../extensions/tests/xa/ServiceUtilsTest.java | 36 ++---------- ...nsions.xa.ActiveMQXAResourceWrapperFactory | 1 + 3 files changed, 47 insertions(+), 45 deletions(-) create mode 100644 artemis-service-extensions/src/test/resources/META-INF/services/org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory diff --git a/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/ServiceUtils.java b/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/ServiceUtils.java index 6452c6ee1c..99a372b5b4 100644 --- a/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/ServiceUtils.java +++ b/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/ServiceUtils.java @@ -16,12 +16,15 @@ */ package org.apache.activemq.artemis.service.extensions; -import javax.transaction.TransactionManager; -import javax.transaction.xa.XAResource; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Iterator; import java.util.Map; import java.util.ServiceLoader; +import javax.transaction.TransactionManager; +import javax.transaction.xa.XAResource; + import org.apache.activemq.artemis.service.extensions.transactions.TransactionManagerLocator; import org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapper; import org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory; @@ -37,7 +40,7 @@ public class ServiceUtils { private static ActiveMQXAResourceWrapperFactory getActiveMQXAResourceWrapperFactory() { if (activeMQXAResourceWrapperFactory == null) { - setActiveMQXAResourceWrapperFactory(ServiceLoader.load(ActiveMQXAResourceWrapperFactory.class)); + activeMQXAResourceWrapperFactory = findActiveMQXAResourceWrapperFactory(); } return activeMQXAResourceWrapperFactory; } @@ -48,11 +51,7 @@ public class ServiceUtils { public static synchronized TransactionManager getTransactionManager() { if (!transactionManagerLoaded) { - Iterator it = ServiceLoader.load(TransactionManagerLocator.class).iterator(); - while (it.hasNext() && transactionManager == null) { - transactionManager = it.next().getTransactionManager(); - } - + transactionManager = findTransactionManager(); if (transactionManager != null) { transactionManagerLoaded = true; } @@ -68,13 +67,39 @@ public class ServiceUtils { transactionManagerLoaded = (transactionManager != null); } - private static void setActiveMQXAResourceWrapperFactory(Iterable iterable) { - if (iterable.iterator().hasNext()) { - activeMQXAResourceWrapperFactory = iterable.iterator().next(); - } - else { - activeMQXAResourceWrapperFactory = new ActiveMQXAResourceWrapperFactoryImpl(); - } + + /** + * Find the first transaction manager loaded from the {@code TransactionManagerLocator} service or {@code null} if none is loaded. + */ + private static TransactionManager findTransactionManager() { + return AccessController.doPrivileged(new PrivilegedAction() { + @Override + public TransactionManager run() { + Iterator it = ServiceLoader.load(TransactionManagerLocator.class, ServiceUtils.class.getClassLoader()).iterator(); + while (it.hasNext() && transactionManager == null) { + transactionManager = it.next().getTransactionManager(); + } + return transactionManager; + } + }); } + /** + * Find the first wrapper factory loaded from the {@code ActiveMQXAResourceWrapperFactory} service or + * use the default {@code ActiveMQXAResourceWrapperFactoryImpl} if none is loaded. + */ + private static ActiveMQXAResourceWrapperFactory findActiveMQXAResourceWrapperFactory() { + return AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ActiveMQXAResourceWrapperFactory run() { + Iterator iterator = ServiceLoader.load(ActiveMQXAResourceWrapperFactory.class, ServiceUtils.class.getClassLoader()).iterator(); + if (iterator.hasNext()) { + return iterator.next(); + } + else { + return new ActiveMQXAResourceWrapperFactoryImpl(); + } + } + }); + } } diff --git a/artemis-service-extensions/src/test/java/org/apache/activemq/artemis/service/extensions/tests/xa/ServiceUtilsTest.java b/artemis-service-extensions/src/test/java/org/apache/activemq/artemis/service/extensions/tests/xa/ServiceUtilsTest.java index 90754bd7b2..5e9db0047c 100644 --- a/artemis-service-extensions/src/test/java/org/apache/activemq/artemis/service/extensions/tests/xa/ServiceUtilsTest.java +++ b/artemis-service-extensions/src/test/java/org/apache/activemq/artemis/service/extensions/tests/xa/ServiceUtilsTest.java @@ -16,44 +16,20 @@ */ package org.apache.activemq.artemis.service.extensions.tests.xa; -import java.lang.reflect.Field; +import static org.jgroups.util.Util.assertTrue; + import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; import org.apache.activemq.artemis.service.extensions.ServiceUtils; -import org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory; -import org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactoryImpl; import org.junit.Test; -import static org.jgroups.util.Util.assertTrue; - public class ServiceUtilsTest { @Test - public void testSetActiveMQXAResourceWrapperFactorySetsDefaultImplWhenNoOther() throws Exception { - List factories = new ArrayList(); - - Method method = ServiceUtils.class.getDeclaredMethod("setActiveMQXAResourceWrapperFactory", Iterable.class); + public void testGetActiveMQXAResourceWrapperFactoryLoadsService() throws Exception { + Method method = ServiceUtils.class.getDeclaredMethod("getActiveMQXAResourceWrapperFactory"); method.setAccessible(true); - method.invoke(null, factories); - - Field field = ServiceUtils.class.getDeclaredField("activeMQXAResourceWrapperFactory"); - field.setAccessible(true); - assertTrue(field.get(null) instanceof ActiveMQXAResourceWrapperFactoryImpl); - } - - @Test - public void testSetActiveMQXAResourceWrapperFactorySetsExtensionImplWhenSupplied() throws Exception { - List factories = new ArrayList(); - factories.add(new MockActiveMQResourceWrapperFactory()); - - Method method = ServiceUtils.class.getDeclaredMethod("setActiveMQXAResourceWrapperFactory", Iterable.class); - method.setAccessible(true); - method.invoke(null, factories); - - Field field = ServiceUtils.class.getDeclaredField("activeMQXAResourceWrapperFactory"); - field.setAccessible(true); - assertTrue(field.get(null) instanceof MockActiveMQResourceWrapperFactory); + Object o = method.invoke(null); + assertTrue(o instanceof MockActiveMQResourceWrapperFactory); } } diff --git a/artemis-service-extensions/src/test/resources/META-INF/services/org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory b/artemis-service-extensions/src/test/resources/META-INF/services/org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory new file mode 100644 index 0000000000..b7781909d5 --- /dev/null +++ b/artemis-service-extensions/src/test/resources/META-INF/services/org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapperFactory @@ -0,0 +1 @@ +org.apache.activemq.artemis.service.extensions.tests.xa.MockActiveMQResourceWrapperFactory \ No newline at end of file