From e44c81672ff4b79887713bea24cc6523374df67a Mon Sep 17 00:00:00 2001 From: David Herberth Date: Sat, 24 Feb 2018 15:59:02 +0100 Subject: [PATCH 1/2] Set classloader for JMX endpoints to application classloader See gh-12209 --- .../actuate/endpoint/jmx/EndpointMBean.java | 37 ++++++++++++++++++- .../endpoint/jmx/JmxEndpointExporter.java | 11 +++++- .../endpoint/jmx/EndpointMBeanTests.java | 16 ++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java index 8a3a79e668e..307cbc0d4c9 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java @@ -29,8 +29,11 @@ import javax.management.MBeanException; import javax.management.MBeanInfo; import javax.management.ReflectionException; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException; import org.springframework.boot.actuate.endpoint.InvocationContext; import org.springframework.boot.actuate.endpoint.SecurityContext; @@ -46,11 +49,15 @@ import org.springframework.util.ClassUtils; * @author Phillip Webb * @since 2.0.0 */ -public class EndpointMBean implements DynamicMBean { +public class EndpointMBean implements DynamicMBean, BeanClassLoaderAware { private static final boolean REACTOR_PRESENT = ClassUtils.isPresent( "reactor.core.publisher.Mono", EndpointMBean.class.getClassLoader()); + private static final Log logger = LogFactory.getLog(EndpointMBean.class); + + private ClassLoader classLoader; + private final JmxOperationResponseMapper responseMapper; private final ExposableJmxEndpoint endpoint; @@ -69,6 +76,11 @@ public class EndpointMBean implements DynamicMBean { this.operations = getOperations(endpoint); } + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + this.classLoader = classLoader; + } + private Map getOperations(ExposableJmxEndpoint endpoint) { Map operations = new HashMap<>(); endpoint.getOperations() @@ -90,7 +102,28 @@ public class EndpointMBean implements DynamicMBean { + "' has no operation named " + actionName; throw new ReflectionException(new IllegalArgumentException(message), message); } - return invoke(operation, params); + ClassLoader previousClassLoader = overrideThreadContextClassLoaderSafe(this.classLoader); + try { + return invoke(operation, params); + } + finally { + overrideThreadContextClassLoaderSafe(previousClassLoader); + } + } + + private static ClassLoader overrideThreadContextClassLoaderSafe(ClassLoader classLoader) { + if (classLoader == null) { + return null; + } + + try { + return ClassUtils.overrideThreadContextClassLoader(classLoader); + } + catch (SecurityException exc) { + // can't set class loader, ignore it and proceed + logger.warn("Unable to override class loader for JMX endpoint."); + } + return null; } private Object invoke(JmxOperation operation, Object[] params) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java index 97f71333dbb..826d745ebf7 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java @@ -29,6 +29,7 @@ import javax.management.ObjectName; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.jmx.JmxException; @@ -42,10 +43,12 @@ import org.springframework.util.Assert; * @author Phillip Webb * @since 2.0.0 */ -public class JmxEndpointExporter implements InitializingBean, DisposableBean { +public class JmxEndpointExporter implements InitializingBean, DisposableBean, BeanClassLoaderAware { private static final Log logger = LogFactory.getLog(JmxEndpointExporter.class); + private ClassLoader classLoader; + private final MBeanServer mBeanServer; private final EndpointObjectNameFactory objectNameFactory; @@ -70,6 +73,11 @@ public class JmxEndpointExporter implements InitializingBean, DisposableBean { this.endpoints = Collections.unmodifiableCollection(endpoints); } + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + this.classLoader = classLoader; + } + @Override public void afterPropertiesSet() { this.registered = register(); @@ -89,6 +97,7 @@ public class JmxEndpointExporter implements InitializingBean, DisposableBean { try { ObjectName name = this.objectNameFactory.getObjectName(endpoint); EndpointMBean mbean = new EndpointMBean(this.responseMapper, endpoint); + mbean.setBeanClassLoader(this.classLoader); this.mBeanServer.registerMBean(mbean, name); return name; } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java index f851e3febc5..7247d4db382 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java @@ -16,6 +16,9 @@ package org.springframework.boot.actuate.endpoint.jmx; +import java.net.URL; +import java.net.URLClassLoader; + import javax.management.Attribute; import javax.management.AttributeList; import javax.management.AttributeNotFoundException; @@ -32,6 +35,7 @@ import reactor.core.publisher.Mono; import org.springframework.beans.FatalBeanException; import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException; import org.springframework.boot.actuate.endpoint.InvocationContext; +import org.springframework.util.ClassUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.instanceOf; @@ -126,6 +130,18 @@ public class EndpointMBeanTests { bean.invoke("missingOperation", NO_PARAMS, NO_SIGNATURE); } + @Test + public void invokeShouldInvokeJmxOperationWithBeanClassLoader() + throws ReflectionException, MBeanException { + TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint( + new TestJmxOperation((arguments) -> ClassUtils.getDefaultClassLoader())); + URLClassLoader beanClassLoader = new URLClassLoader(new URL[]{}, getClass().getClassLoader()); + EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); + bean.setBeanClassLoader(beanClassLoader); + Object result = bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE); + assertThat(result).isEqualTo(beanClassLoader); + } + @Test public void invokeWhenOperationIsInvalidShouldThrowException() throws MBeanException, ReflectionException { From 2be1c8f527e9a4b542c458400222a091b4f0d915 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 14 May 2018 17:34:24 +0200 Subject: [PATCH 2/2] Polish "Set classloader for JMX endpoints to application classloader" Closes gh-12209 --- .../actuate/endpoint/jmx/EndpointMBean.java | 41 +++++++----------- .../endpoint/jmx/JmxEndpointExporter.java | 7 ++-- .../endpoint/jmx/EndpointMBeanTests.java | 42 +++++++++++-------- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java index 307cbc0d4c9..327b1e17dc3 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java @@ -29,11 +29,8 @@ import javax.management.MBeanException; import javax.management.MBeanInfo; import javax.management.ReflectionException; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; -import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.boot.actuate.endpoint.InvalidEndpointRequestException; import org.springframework.boot.actuate.endpoint.InvocationContext; import org.springframework.boot.actuate.endpoint.SecurityContext; @@ -49,37 +46,32 @@ import org.springframework.util.ClassUtils; * @author Phillip Webb * @since 2.0.0 */ -public class EndpointMBean implements DynamicMBean, BeanClassLoaderAware { +public class EndpointMBean implements DynamicMBean { private static final boolean REACTOR_PRESENT = ClassUtils.isPresent( "reactor.core.publisher.Mono", EndpointMBean.class.getClassLoader()); - private static final Log logger = LogFactory.getLog(EndpointMBean.class); - - private ClassLoader classLoader; - private final JmxOperationResponseMapper responseMapper; + private final ClassLoader classLoader; + private final ExposableJmxEndpoint endpoint; private final MBeanInfo info; private final Map operations; - EndpointMBean(JmxOperationResponseMapper responseMapper, + EndpointMBean(JmxOperationResponseMapper responseMapper, ClassLoader classLoader, ExposableJmxEndpoint endpoint) { Assert.notNull(responseMapper, "ResponseMapper must not be null"); Assert.notNull(endpoint, "Endpoint must not be null"); this.responseMapper = responseMapper; + this.classLoader = classLoader; this.endpoint = endpoint; this.info = new MBeanInfoFactory(responseMapper).getMBeanInfo(endpoint); this.operations = getOperations(endpoint); } - @Override - public void setBeanClassLoader(ClassLoader classLoader) { - this.classLoader = classLoader; - } private Map getOperations(ExposableJmxEndpoint endpoint) { Map operations = new HashMap<>(); @@ -102,26 +94,23 @@ public class EndpointMBean implements DynamicMBean, BeanClassLoaderAware { + "' has no operation named " + actionName; throw new ReflectionException(new IllegalArgumentException(message), message); } - ClassLoader previousClassLoader = overrideThreadContextClassLoaderSafe(this.classLoader); + ClassLoader previousClassLoader = overrideThreadContextClassLoader(this.classLoader); try { return invoke(operation, params); } finally { - overrideThreadContextClassLoaderSafe(previousClassLoader); + overrideThreadContextClassLoader(previousClassLoader); } } - private static ClassLoader overrideThreadContextClassLoaderSafe(ClassLoader classLoader) { - if (classLoader == null) { - return null; - } - - try { - return ClassUtils.overrideThreadContextClassLoader(classLoader); - } - catch (SecurityException exc) { - // can't set class loader, ignore it and proceed - logger.warn("Unable to override class loader for JMX endpoint."); + private ClassLoader overrideThreadContextClassLoader(ClassLoader classLoader) { + if (classLoader != null) { + try { + return ClassUtils.overrideThreadContextClassLoader(classLoader); + } + catch (SecurityException ex) { + // can't set class loader, ignore it and proceed + } } return null; } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java index 826d745ebf7..33aa7e64c47 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpointExporter.java @@ -43,7 +43,8 @@ import org.springframework.util.Assert; * @author Phillip Webb * @since 2.0.0 */ -public class JmxEndpointExporter implements InitializingBean, DisposableBean, BeanClassLoaderAware { +public class JmxEndpointExporter + implements InitializingBean, DisposableBean, BeanClassLoaderAware { private static final Log logger = LogFactory.getLog(JmxEndpointExporter.class); @@ -96,8 +97,8 @@ public class JmxEndpointExporter implements InitializingBean, DisposableBean, Be Assert.notNull(endpoint, "Endpoint must not be null"); try { ObjectName name = this.objectNameFactory.getObjectName(endpoint); - EndpointMBean mbean = new EndpointMBean(this.responseMapper, endpoint); - mbean.setBeanClassLoader(this.classLoader); + EndpointMBean mbean = new EndpointMBean(this.responseMapper, this.classLoader, + endpoint); this.mBeanServer.registerMBean(mbean, name); return name; } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java index 7247d4db382..6813552892a 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java @@ -67,19 +67,19 @@ public class EndpointMBeanTests { public void createWhenResponseMapperIsNullShouldThrowException() { this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("ResponseMapper must not be null"); - new EndpointMBean(null, mock(ExposableJmxEndpoint.class)); + new EndpointMBean(null, null, mock(ExposableJmxEndpoint.class)); } @Test public void createWhenEndpointIsNullShouldThrowException() { this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Endpoint must not be null"); - new EndpointMBean(mock(JmxOperationResponseMapper.class), null); + new EndpointMBean(mock(JmxOperationResponseMapper.class), null, null); } @Test public void getMBeanInfoShouldReturnMBeanInfo() { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); MBeanInfo info = bean.getMBeanInfo(); assertThat(info.getDescription()).isEqualTo("MBean operations for endpoint test"); } @@ -87,7 +87,7 @@ public class EndpointMBeanTests { @Test public void invokeShouldInvokeJmxOperation() throws MBeanException, ReflectionException { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); Object result = bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE); assertThat(result).isEqualTo("result"); } @@ -99,7 +99,7 @@ public class EndpointMBeanTests { new TestJmxOperation((arguments) -> { throw new FatalBeanException("test failure"); })); - EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); + EndpointMBean bean = new EndpointMBean(this.responseMapper, null, endpoint); this.thrown.expect(MBeanException.class); this.thrown.expectCause(instanceOf(IllegalStateException.class)); this.thrown.expectMessage("test failure"); @@ -113,7 +113,7 @@ public class EndpointMBeanTests { new TestJmxOperation((arguments) -> { throw new UnsupportedOperationException("test failure"); })); - EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); + EndpointMBean bean = new EndpointMBean(this.responseMapper, null, endpoint); this.thrown.expect(MBeanException.class); this.thrown.expectCause(instanceOf(UnsupportedOperationException.class)); this.thrown.expectMessage("test failure"); @@ -123,7 +123,7 @@ public class EndpointMBeanTests { @Test public void invokeWhenActionNameIsNotAnOperationShouldThrowException() throws MBeanException, ReflectionException { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); this.thrown.expect(ReflectionException.class); this.thrown.expectCause(instanceOf(IllegalArgumentException.class)); this.thrown.expectMessage("no operation named missingOperation"); @@ -133,13 +133,17 @@ public class EndpointMBeanTests { @Test public void invokeShouldInvokeJmxOperationWithBeanClassLoader() throws ReflectionException, MBeanException { + ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader(); TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint( new TestJmxOperation((arguments) -> ClassUtils.getDefaultClassLoader())); - URLClassLoader beanClassLoader = new URLClassLoader(new URL[]{}, getClass().getClassLoader()); - EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); - bean.setBeanClassLoader(beanClassLoader); + URLClassLoader beanClassLoader = new URLClassLoader(new URL[0], + getClass().getClassLoader()); + EndpointMBean bean = new EndpointMBean(this.responseMapper, beanClassLoader, + endpoint); Object result = bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE); assertThat(result).isEqualTo(beanClassLoader); + assertThat(Thread.currentThread().getContextClassLoader()) + .isEqualTo(originalClassLoader); } @Test @@ -154,7 +158,7 @@ public class EndpointMBeanTests { }; TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint(operation); - EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); + EndpointMBean bean = new EndpointMBean(this.responseMapper, null, endpoint); this.thrown.expect(ReflectionException.class); this.thrown.expectCause(instanceOf(IllegalArgumentException.class)); this.thrown.expectMessage("test failure"); @@ -166,7 +170,7 @@ public class EndpointMBeanTests { throws MBeanException, ReflectionException { TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint( new TestJmxOperation((arguments) -> Mono.just("monoResult"))); - EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); + EndpointMBean bean = new EndpointMBean(this.responseMapper, null, endpoint); Object result = bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE); assertThat(result).isEqualTo("monoResult"); } @@ -175,7 +179,7 @@ public class EndpointMBeanTests { public void invokeShouldCallResponseMapper() throws MBeanException, ReflectionException { TestJmxOperationResponseMapper responseMapper = spy(this.responseMapper); - EndpointMBean bean = new EndpointMBean(responseMapper, this.endpoint); + EndpointMBean bean = new EndpointMBean(responseMapper, null, this.endpoint); bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE); verify(responseMapper).mapResponseType(String.class); verify(responseMapper).mapResponse("result"); @@ -184,7 +188,7 @@ public class EndpointMBeanTests { @Test public void getAttributeShouldThrowException() throws AttributeNotFoundException, MBeanException, ReflectionException { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); this.thrown.expect(AttributeNotFoundException.class); this.thrown.expectMessage("EndpointMBeans do not support attributes"); bean.getAttribute("test"); @@ -193,7 +197,7 @@ public class EndpointMBeanTests { @Test public void setAttributeShouldThrowException() throws AttributeNotFoundException, InvalidAttributeValueException, MBeanException, ReflectionException { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); this.thrown.expect(AttributeNotFoundException.class); this.thrown.expectMessage("EndpointMBeans do not support attributes"); bean.setAttribute(new Attribute("test", "test")); @@ -201,18 +205,22 @@ public class EndpointMBeanTests { @Test public void getAttributesShouldReturnEmptyAttributeList() { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); AttributeList attributes = bean.getAttributes(new String[] { "test" }); assertThat(attributes).isEmpty(); } @Test public void setAttributesShouldReturnEmptyAttributeList() { - EndpointMBean bean = new EndpointMBean(this.responseMapper, this.endpoint); + EndpointMBean bean = createEndpointMBean(); AttributeList sourceAttributes = new AttributeList(); sourceAttributes.add(new Attribute("test", "test")); AttributeList attributes = bean.setAttributes(sourceAttributes); assertThat(attributes).isEmpty(); } + private EndpointMBean createEndpointMBean() { + return new EndpointMBean(this.responseMapper, null, this.endpoint); + } + }