From f92ebea9f0512a03c26bb94f5bbe2715bc909ee6 Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Fri, 10 Aug 2012 10:29:09 -0500 Subject: [PATCH 1/4] updates to managed attribute, tests, objectmbean impl and a note to improve id as viewed in jconsole --- .../org/eclipse/jetty/jmx/MBeanContainer.java | 2 + .../org/eclipse/jetty/jmx/ObjectMBean.java | 206 +++++++----------- .../eclipse/jetty/util/log/jmx/LogMBean.java | 14 +- jetty-jmx/src/test/java/com/acme/Derived.java | 4 +- jetty-jmx/src/test/java/com/acme/Managed.java | 8 +- .../test/java/com/acme/jmx/ManagedMBean.java | 3 +- .../eclipse/jetty/jmx/ObjectMBeanTest.java | 111 ++++++++-- .../util/annotation/ManagedAttribute.java | 13 +- .../util/component/AbstractLifeCycle.java | 7 +- .../jetty/util/thread/QueuedThreadPool.java | 23 +- .../eclipse/jetty/util/thread/ThreadPool.java | 6 + 11 files changed, 236 insertions(+), 161 deletions(-) diff --git a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java index 8961a3e6452..4d681e0d2f5 100644 --- a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java +++ b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java @@ -228,6 +228,8 @@ public class MBeanContainer extends AbstractLifeCycle implements Container.Liste * Implementation of Container.Listener interface * * @see org.eclipse.jetty.util.component.Container.Listener#addBean(java.lang.Object) + * + * TODO improve the id property to include better information */ public synchronized void addBean(Object obj) { diff --git a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ObjectMBean.java b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ObjectMBean.java index f4601775a57..ffaeb33bacb 100644 --- a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ObjectMBean.java +++ b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ObjectMBean.java @@ -75,10 +75,10 @@ public class ObjectMBean implements DynamicMBean protected Object _managed; private MBeanInfo _info; - private Map _getters=new HashMap(); - private Map _setters=new HashMap(); - private Map _methods=new HashMap(); - private Set _convert=new HashSet(); + private Map _getters=new HashMap(); + private Map _setters=new HashMap(); + private Map _methods=new HashMap(); + private Set _convert=new HashSet(); private ClassLoader _loader; private MBeanContainer _mbeanContainer; @@ -261,19 +261,6 @@ public class ObjectMBean implements DynamicMBean continue; } - // Process Field Annotations - for (Field field : oClass.getDeclaredFields()) - { - LOG.debug("Checking: " + field.getName()); - ManagedAttribute fieldAnnotation = field.getAnnotation(ManagedAttribute.class); - - if (fieldAnnotation != null) - { - LOG.debug("Field Annotation found for: " + field.getName()); - attributes.add(defineAttribute(field.getName(),fieldAnnotation)); - } - } - // Process Method Annotations for (Method method : oClass.getDeclaredMethods()) @@ -284,7 +271,7 @@ public class ObjectMBean implements DynamicMBean { // TODO sort out how a proper name could get here, its a method name as an attribute at this point. LOG.debug("Attribute Annotation found for: " + method.getName()); - attributes.add(defineAttribute(method.getName(),methodAttributeAnnotation)); + attributes.add(defineAttribute(method,methodAttributeAnnotation)); } ManagedOperation methodOperationAnnotation = method.getAnnotation(ManagedOperation.class); @@ -572,119 +559,80 @@ public class ObjectMBean implements DynamicMBean * * the access is either "RW" or "RO". */ - public MBeanAttributeInfo defineAttribute(String name, ManagedAttribute attributeAnnotation) + public MBeanAttributeInfo defineAttribute(Method method, ManagedAttribute attributeAnnotation) { - //String name = field.getName(); + // determine the name of the managed attribute + String name = attributeAnnotation.name(); + + if ("".equals(name)) + { + name = toVariableName(method.getName()); + } + String description = attributeAnnotation.value(); - boolean writable = !attributeAnnotation.readonly(); + boolean readonly = attributeAnnotation.readonly(); boolean onMBean = attributeAnnotation.proxied(); boolean convert = attributeAnnotation.managed(); String uName = name.substring(0, 1).toUpperCase() + name.substring(1); - Class oClass = onMBean ? this.getClass() : _managed.getClass(); + Class oClass = onMBean ? this.getClass() : _managed.getClass(); - LOG.debug("defineAttribute {} {}:{}:{}:{}",name,onMBean,writable,oClass,description); + LOG.debug("defineAttribute {} {}:{}:{}:{}",name,onMBean,readonly,oClass,description); - Class type = null; - Method getter = null; + Class type = null; Method setter = null; + + type = method.getReturnType(); - String declaredGetter = attributeAnnotation.getter(); - String declaredSetter = attributeAnnotation.setter(); - - - Method[] methods = oClass.getMethods(); - for (int m = 0; m < methods.length; m++) + + // dig out a setter if one exists + if (!readonly) { - if ((methods[m].getModifiers() & Modifier.PUBLIC) == 0) - continue; + String declaredSetter = attributeAnnotation.setter(); - // Check if it is a declared getter - if (methods[m].getName().equals(declaredGetter) && methods[m].getParameterTypes().length == 0) + LOG.debug("DeclaredSetter:" + declaredSetter); + Method[] methods = oClass.getMethods(); + for (int m = 0; m < methods.length; m++) { - if (getter != null) - { - LOG.warn("Multiple mbean getters for attr " + name+ " in "+oClass); + if ((methods[m].getModifiers() & Modifier.PUBLIC) == 0) continue; - } - getter = methods[m]; - if (type != null && !type.equals(methods[m].getReturnType())) + + if (!"".equals(declaredSetter)) { - LOG.warn("Type conflict for mbean attr " + name+ " in "+oClass); - continue; + + // look for a declared setter + if (methods[m].getName().equals(declaredSetter) && methods[m].getParameterTypes().length == 1) + { + if (setter != null) + { + LOG.warn("Multiple setters for mbean attr " + name + " in " + oClass); + continue; + } + setter = methods[m]; + if ( !type.equals(methods[m].getParameterTypes()[0])) + { + LOG.warn("Type conflict for mbean attr " + name + " in " + oClass); + continue; + } + LOG.debug("Declared Setter: " + declaredSetter); + } } - type = methods[m].getReturnType(); - LOG.debug("Declared Getter: " + declaredGetter); - } - - // Look for a getter - if (methods[m].getName().equals("get" + uName) && methods[m].getParameterTypes().length == 0) - { - if (getter != null) + // look for a setter + if ( methods[m].getName().equals("set" + uName) && methods[m].getParameterTypes().length == 1) { - LOG.warn("Multiple mbean getters for attr " + name+ " in "+oClass); - continue; - } - getter = methods[m]; - if (type != null && !type.equals(methods[m].getReturnType())) - { - LOG.warn("Type conflict for mbean attr " + name+ " in "+oClass); - continue; - } - type = methods[m].getReturnType(); - } - - // Look for an is getter - if (methods[m].getName().equals("is" + uName) && methods[m].getParameterTypes().length == 0) - { - if (getter != null) - { - LOG.warn("Multiple mbean getters for attr " + name+ " in "+oClass); - continue; - } - getter = methods[m]; - if (type != null && !type.equals(methods[m].getReturnType())) - { - LOG.warn("Type conflict for mbean attr " + name+ " in "+oClass); - continue; - } - type = methods[m].getReturnType(); - } - - // look for a declared setter - if (writable && methods[m].getName().equals(declaredSetter) && methods[m].getParameterTypes().length == 1) - { - if (setter != null) - { - LOG.warn("Multiple setters for mbean attr " + name+ " in "+oClass); - continue; - } - setter = methods[m]; - if (type != null && !type.equals(methods[m].getParameterTypes()[0])) - { - LOG.warn("Type conflict for mbean attr " + name+ " in "+oClass); - continue; - } - LOG.debug("Declared Setter: " + declaredSetter); - type = methods[m].getParameterTypes()[0]; - } - - // look for a setter - if (writable && methods[m].getName().equals("set" + uName) && methods[m].getParameterTypes().length == 1) - { - if (setter != null) - { - LOG.warn("Multiple setters for mbean attr " + name+ " in "+oClass); - continue; + if (setter != null) + { + LOG.warn("Multiple setters for mbean attr " + name + " in " + oClass); + continue; + } + setter = methods[m]; + if ( !type.equals(methods[m].getParameterTypes()[0])) + { + LOG.warn("Type conflict for mbean attr " + name + " in " + oClass); + continue; + } } - setter = methods[m]; - if (type != null && !type.equals(methods[m].getParameterTypes()[0])) - { - LOG.warn("Type conflict for mbean attr " + name+ " in "+oClass); - continue; - } - type = methods[m].getParameterTypes()[0]; } } @@ -704,16 +652,10 @@ public class ObjectMBean implements DynamicMBean LOG.debug("passed convert checks {} for type {}", name, type); } - if (getter == null && setter == null) - { - LOG.warn("No mbean getter or setters found for {} in {}", name, oClass); - return null; - } - try { // Remember the methods - _getters.put(name, getter); + _getters.put(name, method); _setters.put(name, setter); MBeanAttributeInfo info=null; @@ -723,16 +665,16 @@ public class ObjectMBean implements DynamicMBean if (type.isArray()) { - info= new MBeanAttributeInfo(name,OBJECT_NAME_ARRAY_CLASS,description,getter!=null,setter!=null,getter!=null&&getter.getName().startsWith("is")); + info= new MBeanAttributeInfo(name,OBJECT_NAME_ARRAY_CLASS,description,true,setter!=null,method.getName().startsWith("is")); } else { - info= new MBeanAttributeInfo(name,OBJECT_NAME_CLASS,description,getter!=null,setter!=null,getter!=null&&getter.getName().startsWith("is")); + info= new MBeanAttributeInfo(name,OBJECT_NAME_CLASS,description,true,setter!=null,method.getName().startsWith("is")); } } else { - info= new MBeanAttributeInfo(name,description,getter,setter); + info= new MBeanAttributeInfo(name,description,method,setter); } return info; @@ -834,5 +776,25 @@ public class ObjectMBean implements DynamicMBean } } + + + protected String toVariableName( String methodName ) + { + String variableName = methodName; + + if ( methodName.startsWith("get") || methodName.startsWith("set") ) + { + variableName = variableName.substring(3); + } + else if ( methodName.startsWith("is") ) + { + variableName = variableName.substring(2); + } + + variableName = variableName.substring(0,1).toLowerCase() + variableName.substring(1); + + return variableName; + } + } diff --git a/jetty-jmx/src/main/java/org/eclipse/jetty/util/log/jmx/LogMBean.java b/jetty-jmx/src/main/java/org/eclipse/jetty/util/log/jmx/LogMBean.java index b5fa44b9c8e..415f7c90865 100644 --- a/jetty-jmx/src/main/java/org/eclipse/jetty/util/log/jmx/LogMBean.java +++ b/jetty-jmx/src/main/java/org/eclipse/jetty/util/log/jmx/LogMBean.java @@ -17,11 +17,16 @@ import java.util.ArrayList; import java.util.List; import org.eclipse.jetty.jmx.ObjectMBean; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.annotation.ManagedOperation; +import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.log.Log; /* ------------------------------------------------------------ */ /** */ +@ManagedObject("Jetty Logging") public class LogMBean extends ObjectMBean { @@ -30,18 +35,21 @@ public class LogMBean extends ObjectMBean super(managedObject); } + @ManagedAttribute(value="list of instantiated loggers") public List getLoggers() { List keySet = new ArrayList(Log.getLoggers().keySet()); return keySet; } - public boolean isDebugEnabled(String logger) + @ManagedOperation(value="true if debug enabled for the given logger") + public boolean isDebugEnabled(@Name("logger") String logger) { return Log.getLogger(logger).isDebugEnabled(); } - - public void setDebugEnabled(String logger, Boolean enabled) + + @ManagedOperation(value="Set debug enabled for given logger") + public void setDebugEnabled(@Name("logger")String logger, @Name("enabled") Boolean enabled) { Log.getLogger(logger).setDebugEnabled(enabled); } diff --git a/jetty-jmx/src/test/java/com/acme/Derived.java b/jetty-jmx/src/test/java/com/acme/Derived.java index 4fc9f73627a..49fdf0a66d0 100644 --- a/jetty-jmx/src/test/java/com/acme/Derived.java +++ b/jetty-jmx/src/test/java/com/acme/Derived.java @@ -21,12 +21,11 @@ import org.eclipse.jetty.util.annotation.Name; @ManagedObject(value="Test the mbean stuff", wrapper="com.acme.jmx.DerivedMBean") public class Derived extends Base implements Signature { - @ManagedAttribute(value="The full name of something", getter="getFullName", setter="setFullName") String fname="Full Name"; - @ManagedAttribute( value="sample managed object") Managed managedInstance = new Managed(); + @ManagedAttribute(value="The full name of something", name="fname", setter="setFullName") public String getFullName() { return fname; @@ -54,6 +53,7 @@ public class Derived extends Base implements Signature return "bad"; } + @ManagedAttribute( value="sample managed object") public Managed getManagedInstance() { return managedInstance; diff --git a/jetty-jmx/src/test/java/com/acme/Managed.java b/jetty-jmx/src/test/java/com/acme/Managed.java index 8cc83370142..e79ad5a3462 100644 --- a/jetty-jmx/src/test/java/com/acme/Managed.java +++ b/jetty-jmx/src/test/java/com/acme/Managed.java @@ -6,9 +6,9 @@ import org.eclipse.jetty.util.annotation.ManagedObject; @ManagedObject(value="Managed Object", wrapper="com.acme.jmx.ManagedMBean") public class Managed { - @ManagedAttribute("Managed Attribute") String managed = "foo"; + @ManagedAttribute("Managed Attribute") public String getManaged() { return managed; @@ -19,4 +19,10 @@ public class Managed this.managed = managed; } + + public String bad() + { + return "bad"; + } + } diff --git a/jetty-jmx/src/test/java/com/acme/jmx/ManagedMBean.java b/jetty-jmx/src/test/java/com/acme/jmx/ManagedMBean.java index c78145e0b44..f71bf0c5b8e 100644 --- a/jetty-jmx/src/test/java/com/acme/jmx/ManagedMBean.java +++ b/jetty-jmx/src/test/java/com/acme/jmx/ManagedMBean.java @@ -6,6 +6,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; import com.acme.Derived; +import com.acme.Managed; @ManagedObject("Managed MBean Wrapper") public class ManagedMBean extends ObjectMBean @@ -18,7 +19,7 @@ public class ManagedMBean extends ObjectMBean @ManagedOperation(value="test of proxy operations", managed=true) public String good() { - return "not " + ((Derived)_managed).bad(); + return "not managed " + ((Managed)_managed).bad(); } @ManagedAttribute(value="test of proxy attributes", getter="goop", proxied=true) diff --git a/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java b/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java index a97562b8b83..8ce6d736eca 100644 --- a/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java +++ b/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java @@ -1,4 +1,4 @@ -// ======================================================================== +// =====7=================================================================== // Copyright (c) 2004-2009 Mort Bay Consulting Pty. Ltd. // ------------------------------------------------------------------------ // All rights reserved. This program and the accompanying materials @@ -25,7 +25,10 @@ import junit.framework.Assert; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.After; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -39,15 +42,15 @@ public class ObjectMBeanTest private static MBeanContainer container; - @BeforeClass - public static void beforeClass() throws Exception + @Before + public void beforeClass() throws Exception { container = new MBeanContainer(ManagementFactory.getPlatformMBeanServer()); container.start(); } - @AfterClass - public static void afterClass() throws Exception + @After + public void afterClass() throws Exception { container.stop(); container = null; @@ -57,7 +60,7 @@ public class ObjectMBeanTest * this test uses the com.acme.Derived test classes */ @Test - public void testMbeanInfo() throws Exception + public void testDerivedAttributes() throws Exception { Derived derived = new Derived(); @@ -67,8 +70,8 @@ public class ObjectMBeanTest mbean.setMBeanContainer(container); managed.setMBeanContainer(container); - container.addBean(mbean); - container.addBean(managed); + container.addBean(derived); + container.addBean(derived.getManagedInstance()); MBeanInfo toss = managed.getMBeanInfo(); @@ -79,15 +82,15 @@ public class ObjectMBeanTest Assert.assertEquals("name does not match", "com.acme.Derived", info.getClassName()); Assert.assertEquals("description does not match", "Test the mbean stuff", info.getDescription()); - for ( MBeanAttributeInfo i : info.getAttributes()) - { - LOG.debug(i.toString()); - } + //for ( MBeanAttributeInfo i : info.getAttributes()) + //{ + // LOG.debug(i.toString()); + //} /* - * 6 attributes from lifecycle and 2 from Derived and 1 from MBean + * 1 attribute from lifecycle and 2 from Derived and 1 from MBean */ - Assert.assertEquals("attribute count does not match", 9, info.getAttributes().length); + Assert.assertEquals("attribute count does not match", 4, info.getAttributes().length); Assert.assertEquals("attribute values does not match", "Full Name", mbean.getAttribute("fname") ); @@ -95,10 +98,25 @@ public class ObjectMBeanTest Assert.assertEquals("set attribute value does not match", "Fuller Name", mbean.getAttribute("fname") ); - Assert.assertEquals("proxy attribute values do not match", "goop", mbean.getAttribute("goop") ); + Assert.assertEquals("proxy attribute values do not match", "goop", mbean.getAttribute("goop") ); + + //Thread.sleep(100000); + } + + @Test + public void testDerivedOperations() throws Exception + { + Derived derived = new Derived(); + ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived); + mbean.setMBeanContainer(container); + + container.addBean(derived); + + MBeanInfo info = mbean.getMBeanInfo(); + Assert.assertEquals("operation count does not match", 5, info.getOperations().length); - + MBeanOperationInfo[] opinfos = info.getOperations(); boolean publish = false; boolean doodle = false; @@ -106,9 +124,7 @@ public class ObjectMBeanTest for ( int i = 0 ; i < opinfos.length; ++i ) { MBeanOperationInfo opinfo = opinfos[i]; - - LOG.debug(opinfo.getName()); - + if ("publish".equals(opinfo.getName())) { publish = true; @@ -126,6 +142,7 @@ public class ObjectMBeanTest Assert.assertEquals("parameter name doesn't match", "doodle", pinfos[0].getName()); } + // This is a proxied operation on the JMX wrapper if ("good".equals(opinfo.getName())) { good = true; @@ -138,11 +155,63 @@ public class ObjectMBeanTest Assert.assertTrue("publish operation was not not found", publish); Assert.assertTrue("doodle operation was not not found", doodle); Assert.assertTrue("good operation was not not found", good); - - // TODO sort out why this is not working...something off in Bean vs MBean ism's + + } + + @Test + public void testDerivedObjectAttributes() throws Exception + { + Derived derived = new Derived(); + ObjectMBean mbean = (ObjectMBean)ObjectMBean.mbeanFor(derived); + + ObjectMBean managed = (ObjectMBean)ObjectMBean.mbeanFor(derived.getManagedInstance()); + mbean.setMBeanContainer(container); + managed.setMBeanContainer(container); + + Assert.assertNotNull(mbean.getMBeanInfo()); + + container.addBean(derived); + container.addBean(derived.getManagedInstance()); Managed managedInstance = (Managed)mbean.getAttribute("managedInstance"); Assert.assertNotNull(managedInstance); Assert.assertEquals("managed instance returning nonsense", "foo", managedInstance.getManaged()); + } + + @Test + public void testThreadPool() throws Exception + { + QueuedThreadPool qtp = new QueuedThreadPool(); + + ObjectMBean bqtp = (ObjectMBean)ObjectMBean.mbeanFor(qtp); + + bqtp.getMBeanInfo(); + + + + container.addBean(qtp); + + + Thread.sleep(10000000); + + } + + @Test + public void testMethodNameMining() throws Exception + { + ObjectMBean mbean = new ObjectMBean(new Derived()); + + Assert.assertEquals("fullName",mbean.toVariableName("getFullName")); + Assert.assertEquals("fullName",mbean.toVariableName("getfullName")); + Assert.assertEquals("fullName",mbean.toVariableName("isFullName")); + Assert.assertEquals("fullName",mbean.toVariableName("isfullName")); + Assert.assertEquals("fullName",mbean.toVariableName("setFullName")); + Assert.assertEquals("fullName",mbean.toVariableName("setfullName")); + Assert.assertEquals("fullName",mbean.toVariableName("FullName")); + Assert.assertEquals("fullName",mbean.toVariableName("fullName")); + } + + } + diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/annotation/ManagedAttribute.java b/jetty-util/src/main/java/org/eclipse/jetty/util/annotation/ManagedAttribute.java index 3e0279cbf6b..c5cf94f6eae 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/annotation/ManagedAttribute.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/annotation/ManagedAttribute.java @@ -20,16 +20,23 @@ import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) @Documented -@Target( { ElementType.METHOD, ElementType.FIELD } ) +@Target( { ElementType.METHOD } ) public @interface ManagedAttribute { /** - * Description of the Managed Object + * Description of the Managed Attribute * - * @return + * @returngit checkout */ String value() default "Not Specified"; + /** + * name to use for the attribute + * + * @return the name of the attribute + */ + String name() default ""; + /** * Is the managed field read-only? * diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java index ab00ce5b044..24685f8c805 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java @@ -30,17 +30,11 @@ public abstract class AbstractLifeCycle implements LifeCycle { private static final Logger LOG = Log.getLogger(AbstractLifeCycle.class); - @ManagedAttribute(value="instance is stopped", readonly=true, getter="isStopped") public static final String STOPPED="STOPPED"; - @ManagedAttribute(value="instance is failed", readonly=true, getter="isFailed") public static final String FAILED="FAILED"; - @ManagedAttribute(value="instance is starting", readonly=true, getter="isStarting") public static final String STARTING="STARTING"; - @ManagedAttribute(value="instance is started", readonly=true, getter="isStarted") public static final String STARTED="STARTED"; - @ManagedAttribute(value="instance is stopping", readonly=true, getter="isStopping") public static final String STOPPING="STOPPING"; - @ManagedAttribute(value="instance is running", readonly=true, getter="isRunning") public static final String RUNNING="RUNNING"; private final CopyOnWriteArrayList _listeners=new CopyOnWriteArrayList(); @@ -139,6 +133,7 @@ public abstract class AbstractLifeCycle implements LifeCycle _listeners.remove(listener); } + @ManagedAttribute(value="Lifecycle State for this instance", readonly=true) public String getState() { switch(_state) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index e7743e1c7ff..8928b0192c0 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -28,6 +28,10 @@ import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.annotation.ManagedOperation; +import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.AggregateLifeCycle; import org.eclipse.jetty.util.component.Dumpable; @@ -36,6 +40,7 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; +@ManagedObject("A thread pool with no max bound by default") public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPool, Dumpable { private static final Logger LOG = Log.getLogger(QueuedThreadPool.class); @@ -46,14 +51,20 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo private final ConcurrentLinkedQueue _threads=new ConcurrentLinkedQueue(); private final Object _joinLock = new Object(); private BlockingQueue _jobs; + private String _name; private int _maxIdleTimeMs=60000; + private int _maxThreads; + private int _minThreads; private int _maxQueued=-1; + private int _priority=Thread.NORM_PRIORITY; + private boolean _daemon=false; private int _maxStopTime=100; + private boolean _detailedDump=false; /* ------------------------------------------------------------------- */ @@ -312,6 +323,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo * @return minimum number of threads. */ @Override + @ManagedAttribute("minimum number of threads in the pool") public int getMinThreads() { return _minThreads; @@ -321,6 +333,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo /** * @return The name of the BoundedThreadPool. */ + @ManagedAttribute("name of the thread pool") public String getName() { return _name; @@ -330,6 +343,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo /** Get the priority of the pool threads. * @return the priority of the pool threads. */ + @ManagedAttribute("priority of threads in the pool") public int getThreadsPriority() { return _priority; @@ -339,12 +353,14 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo /** * Delegated to the named or anonymous Pool. */ + @ManagedAttribute("thead pool using a daemon thread") public boolean isDaemon() { return _daemon; } /* ------------------------------------------------------------ */ + @ManagedAttribute("full stack detail on dump output") public boolean isDetailedDump() { return _detailedDump; @@ -471,6 +487,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo /* ------------------------------------------------------------ */ @Override + @ManagedOperation("dump thread state") public String dump() { return AggregateLifeCycle.dump(this); @@ -658,7 +675,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo * @param id The thread ID to interrupt. * @return true if the thread was found and interrupted. */ - public boolean interruptThread(long id) + @ManagedOperation("interrupt a pool thread") + public boolean interruptThread(@Name("id") long id) { for (Thread thread: _threads) { @@ -676,7 +694,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo * @param id The thread ID to interrupt. * @return true if the thread was found and interrupted. */ - public String dumpThread(long id) + @ManagedOperation("dump a pool thread stack") + public String dumpThread(@Name("id") long id) { for (Thread thread: _threads) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java index 6e3b8ea493e..28ac01ff8d1 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java @@ -15,6 +15,8 @@ package org.eclipse.jetty.util.thread; import java.util.concurrent.Executor; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.LifeCycle; /* ------------------------------------------------------------ */ @@ -22,6 +24,7 @@ import org.eclipse.jetty.util.component.LifeCycle; * * */ +@ManagedObject("Pool of Threads") public interface ThreadPool extends Executor { /* ------------------------------------------------------------ */ @@ -41,18 +44,21 @@ public interface ThreadPool extends Executor /** * @return The total number of threads currently in the pool */ + @ManagedAttribute("number of threads in pool") public int getThreads(); /* ------------------------------------------------------------ */ /** * @return The number of idle threads in the pool */ + @ManagedAttribute("number of idle threads in pool") public int getIdleThreads(); /* ------------------------------------------------------------ */ /** * @return True if the pool is low on threads */ + @ManagedAttribute("indicates the pool is low on available threads") public boolean isLowOnThreads(); From 4d74adc1e576041dfee6816f93c47c5e41a99078 Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Fri, 10 Aug 2012 10:48:44 -0500 Subject: [PATCH 2/4] ignore test --- .../src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java b/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java index 8ce6d736eca..4e88e029c78 100644 --- a/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java +++ b/jetty-jmx/src/test/java/org/eclipse/jetty/jmx/ObjectMBeanTest.java @@ -30,6 +30,7 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import com.acme.Derived; @@ -180,6 +181,7 @@ public class ObjectMBeanTest } @Test + @Ignore("ignore, used in testing jconsole atm") public void testThreadPool() throws Exception { QueuedThreadPool qtp = new QueuedThreadPool(); From 136f7924fdcd20d7410837337f1044ba270bf8be Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 10 Aug 2012 15:51:27 +0200 Subject: [PATCH 3/4] Jetty9 - Refactored usage of components that were using custom stop timeout to use the get/setStopTimeout() methods inherited from AbstractLifeCycle. --- .../jetty/deploy/test/XmlConfiguredJetty.java | 3 +- jetty-deploy/src/test/resources/jetty.xml | 2 +- .../org/eclipse/jetty/io/SelectorManager.java | 10 +- .../jetty-osgi-boot/jettyhome/etc/jetty.xml | 6 +- .../jetty-rewrite.xml | 2 +- .../src/main/config/etc/jetty-proxy.xml | 2 +- jetty-server/src/main/config/etc/jetty.xml | 2 +- .../java/org/eclipse/jetty/server/Server.java | 28 ++-- .../jetty/server/handler/ContextHandler.java | 91 ++++++------ .../util/component/AbstractLifeCycle.java | 3 +- .../util/component/AggregateLifeCycle.java | 14 ++ .../jetty/util/thread/QueuedThreadPool.java | 131 +++--------------- .../util/thread/QueuedThreadPoolTest.java | 61 ++++---- .../server/WebSocketLoadRFC6455Test.java | 9 +- .../src/main/webapp/WEB-INF/jetty.xml | 2 +- .../src/test/resources/DefaultHandler.xml | 2 +- .../src/test/resources/RFC2616Base.xml | 2 +- 17 files changed, 138 insertions(+), 232 deletions(-) diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java index 9b8abcc23d3..3c46cb02d5c 100644 --- a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java @@ -378,8 +378,7 @@ public class XmlConfiguredJetty Assert.assertEquals("Server load count",1,serverCount); this._server = foundServer; - this._server.setGracefulShutdown(10); - + this._server.setStopTimeout(10); } public void removeContext(String name) diff --git a/jetty-deploy/src/test/resources/jetty.xml b/jetty-deploy/src/test/resources/jetty.xml index 4c1734b87f0..785ca813206 100644 --- a/jetty-deploy/src/test/resources/jetty.xml +++ b/jetty-deploy/src/test/resources/jetty.xml @@ -123,6 +123,6 @@ true true true - 1000 + 1000 diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 6b1d2d74e78..4ead5b4802f 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -280,9 +280,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa @Override protected void doStop() throws Exception { - Stop task = new Stop(); - submit(task); - task.await(getStopTimeout(), TimeUnit.MILLISECONDS); + Stop stop = new Stop(); + submit(stop); + stop.await(getStopTimeout()); } /** @@ -677,11 +677,11 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } } - public boolean await(long timeout, TimeUnit unit) + public boolean await(long timeout) { try { - return latch.await(timeout, unit); + return latch.await(timeout, TimeUnit.MILLISECONDS); } catch (InterruptedException x) { diff --git a/jetty-osgi/jetty-osgi-boot/jettyhome/etc/jetty.xml b/jetty-osgi/jetty-osgi-boot/jettyhome/etc/jetty.xml index 30f8ea9d186..a9215884e81 100644 --- a/jetty-osgi/jetty-osgi-boot/jettyhome/etc/jetty.xml +++ b/jetty-osgi/jetty-osgi-boot/jettyhome/etc/jetty.xml @@ -25,7 +25,7 @@ - + @@ -52,11 +52,11 @@ true true true - 1000 + 1000 false false - + diff --git a/jetty-rewrite/src/test/resources/org.mortbay.jetty.rewrite.handler/jetty-rewrite.xml b/jetty-rewrite/src/test/resources/org.mortbay.jetty.rewrite.handler/jetty-rewrite.xml index f5b400133a2..81d3caa7de4 100644 --- a/jetty-rewrite/src/test/resources/org.mortbay.jetty.rewrite.handler/jetty-rewrite.xml +++ b/jetty-rewrite/src/test/resources/org.mortbay.jetty.rewrite.handler/jetty-rewrite.xml @@ -276,6 +276,6 @@ true true true - 1000 + 1000 diff --git a/jetty-server/src/main/config/etc/jetty-proxy.xml b/jetty-server/src/main/config/etc/jetty-proxy.xml index 3706329ec9f..18c2e7b2211 100644 --- a/jetty-server/src/main/config/etc/jetty-proxy.xml +++ b/jetty-server/src/main/config/etc/jetty-proxy.xml @@ -57,6 +57,6 @@ true true true - 1000 + 1000 diff --git a/jetty-server/src/main/config/etc/jetty.xml b/jetty-server/src/main/config/etc/jetty.xml index dee77a02b21..afaaa184a07 100644 --- a/jetty-server/src/main/config/etc/jetty.xml +++ b/jetty-server/src/main/config/etc/jetty.xml @@ -70,7 +70,7 @@ true true true - 1000 + 1000 false false diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 089dbfdc3b2..651a54e020f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpGenerator; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.util.ArrayUtil; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.AttributesMap; import org.eclipse.jetty.util.MultiException; @@ -71,7 +70,6 @@ public class Server extends HandlerWrapper implements Attributes private SessionIdManager _sessionIdManager; private boolean _sendServerVersion = true; //send Server: header private boolean _sendDateHeader = false; //send Date: header - private int _graceful=0; private boolean _stopAtShutdown; private boolean _dumpAfterStart=false; private boolean _dumpBeforeStop=false; @@ -306,8 +304,9 @@ public class Server extends HandlerWrapper implements Attributes MultiException mex=new MultiException(); - long gracefulTimeout = getGracefulShutdown(); - if (gracefulTimeout>0) + // TODO: review this logic ... + long stopTimeout = getStopTimeout(); + if (stopTimeout>0) { for (Connector connector : _connectors) { @@ -321,9 +320,9 @@ public class Server extends HandlerWrapper implements Attributes { Graceful graceful = (Graceful)context; LOG.info("Graceful shutdown {}", graceful); - graceful.setShutdown(true); + graceful.shutdown(); } - Thread.sleep(gracefulTimeout); + Thread.sleep(stopTimeout); } for (Connector connector : _connectors) @@ -564,15 +563,6 @@ public class Server extends HandlerWrapper implements Attributes _attributes.setAttribute(name, attribute); } - /* ------------------------------------------------------------ */ - /** - * @return the graceful - */ - public int getGracefulShutdown() - { - return _graceful; - } - /* ------------------------------------------------------------ */ /** * Set graceful shutdown timeout. If set, the internal doStop() method will not immediately stop the @@ -585,7 +575,7 @@ public class Server extends HandlerWrapper implements Attributes */ public void setGracefulShutdown(int timeoutMS) { - _graceful=timeoutMS; + // TODO } /* ------------------------------------------------------------ */ @@ -618,11 +608,13 @@ public class Server extends HandlerWrapper implements Attributes /* ------------------------------------------------------------ */ /* A handler that can be gracefully shutdown. * Called by doStop if a {@link #setGracefulShutdown} period is set. - * TODO move this somewhere better + * TODO: this interface should be part of a restructuring of how we manage the lifecycle of components + * TODO: it should extend LifeCycle rather than Handler, for example, and should play in concert with + * TODO: LifeCycle.stop() so that stop==shutdown+await(stopTimeout) to keep the stop semantic. */ public interface Graceful extends Handler { - public void setShutdown(boolean shutdown); + public void shutdown(); } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 941e4fc695f..43c5c8a53b0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; - import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterRegistration; @@ -88,13 +87,13 @@ import org.eclipse.jetty.util.resource.Resource; *

* The maximum size of a form that can be processed by this context is controlled by the system properties org.eclipse.jetty.server.Request.maxFormKeys * and org.eclipse.jetty.server.Request.maxFormContentSize. These can also be configured with {@link #setMaxFormContentSize(int)} and {@link #setMaxFormKeys(int)} - * + * * @org.apache.xbean.XBean description="Creates a basic HTTP context" */ @Managed("URI Context") public class ContextHandler extends ScopedHandler implements Attributes, Server.Graceful { - + private static final Logger LOG = Log.getLogger(ContextHandler.class); private static final ThreadLocal __context = new ThreadLocal(); @@ -121,44 +120,44 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. private final AttributesMap _attributes; private final AttributesMap _contextAttributes; - + @Managed("Initial Parameter map for the context") private final Map _initParams; - + private ClassLoader _classLoader; private String _contextPath = "/"; - + @Managed(value="Display name of the Context", readonly=true) private String _displayName; - + private Resource _baseResource; private MimeTypes _mimeTypes; private Map _localeEncodingMap; - + @Managed("Partial URIs of directory welcome files") private String[] _welcomeFiles; - + @Managed(value="The error handler to use for the context", managed=true) private ErrorHandler _errorHandler; - + @Managed("Virtual hosts accepted by the context") private String[] _vhosts; - + private Set _connectors; private EventListener[] _eventListeners; private Logger _logger; - + @Managed("Checks if the /context is not redirected to /context/") private boolean _allowNullPathInfo; - + private int _maxFormKeys = Integer.getInteger("org.eclipse.jetty.server.Request.maxFormKeys",1000).intValue(); - + @Managed("The maximum content size") private int _maxFormContentSize = Integer.getInteger("org.eclipse.jetty.server.Request.maxFormContentSize",200000).intValue(); - + @Managed("True if URLs are compacted to replace the multiple '/'s with a single '/'") private boolean _compactPath = false; - + private boolean _aliases = false; private Object _contextListeners; @@ -170,7 +169,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. @Managed("False if this context is accepting new requests. True for graceful shutdown, which allows existing requests to complete") private boolean _shutdown = false; - + private boolean _available = true; private volatile int _availability; // 0=STOPPED, 1=AVAILABLE, 2=SHUTDOWN, 3=UNAVAILABLE @@ -608,14 +607,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. { setEventListeners((EventListener[])ArrayUtil.addToArray(getEventListeners(),listener,EventListener.class)); } - - + + /** * Apply any necessary restrictions on a programmatically added * listener. - * + * * Superclasses should implement. - * + * * @param listener */ public void restrictEventListener (EventListener listener) @@ -639,15 +638,13 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. * Set shutdown status. This field allows for graceful shutdown of a context. A started context may be put into non accepting state so that existing * requests can complete, but no new requests are accepted. * - * @param shutdown - * true if this context is (not?) accepting new requests */ - public void setShutdown(boolean shutdown) + public void shutdown() { synchronized (this) { - _shutdown = shutdown; - _availability = isRunning()?(_shutdown?__SHUTDOWN:_available?__AVAILABLE:__UNAVAILABLE):__STOPPED; + _shutdown = true; + _availability = isRunning() ? __SHUTDOWN : __STOPPED; } } @@ -796,7 +793,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. { l.contextDestroyed(e); } - + /* ------------------------------------------------------------ */ /* * @see org.eclipse.thread.AbstractLifeCycle#doStop() @@ -968,7 +965,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. target = URIUtil.compactPath(target); if (!checkContext(target,baseRequest,response)) return; - + if (target.length() > _contextPath.length()) { if (_contextPath.length() > 1) @@ -1156,17 +1153,17 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. /* ------------------------------------------------------------ */ /** * Check the target. Called by {@link #handle(String, Request, HttpServletRequest, HttpServletResponse)} when a target within a context is determined. If - * the target is protected, 404 is returned. + * the target is protected, 404 is returned. */ /* ------------------------------------------------------------ */ public boolean isProtectedTarget(String target) { if (target == null || _protectedTargets == null) return false; - + while (target.startsWith("//")) target=URIUtil.compactPath(target); - + boolean isProtected = false; int i=0; while (!isProtected && i<_protectedTargets.length) @@ -1175,8 +1172,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. } return isProtected; } - - + + public void setProtectedTargets (String[] targets) { if (targets == null) @@ -1184,21 +1181,21 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. _protectedTargets = null; return; } - + _protectedTargets = new String[targets.length]; System.arraycopy(targets, 0, _protectedTargets, 0, targets.length); } - + public String[] getProtectedTargets () { if (_protectedTargets == null) return null; - + String[] tmp = new String[_protectedTargets.length]; System.arraycopy(_protectedTargets, 0, tmp, 0, _protectedTargets.length); return tmp; } - + /* ------------------------------------------------------------ */ /* @@ -1795,7 +1792,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. { return 3; } - + /* ------------------------------------------------------------ */ /* @@ -2293,7 +2290,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. { if (!_enabled) throw new UnsupportedOperationException(); - + try { Class clazz = _classLoader==null?Loader.loadClass(ContextHandler.class,className):_classLoader.loadClass(className); @@ -2307,7 +2304,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. @Override public void addListener(T t) - { + { if (!_enabled) throw new UnsupportedOperationException(); ContextHandler.this.addEventListener(t); @@ -2315,7 +2312,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. @Override public void addListener(Class listenerClass) - { + { if (!_enabled) throw new UnsupportedOperationException(); @@ -2371,12 +2368,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. { _majorVersion = v; } - + public void setEffectiveMinorVersion (int v) { _minorVersion = v; } - + @Override public JspConfigDescriptor getJspConfigDescriptor() { @@ -2386,9 +2383,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. public void setJspConfigDescriptor(JspConfigDescriptor d) { - + } - + @Override public void declareRoles(String... roleNames) { @@ -2396,9 +2393,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. throw new IllegalStateException (); if (!_enabled) throw new UnsupportedOperationException(); - + // TODO Auto-generated method stub - + } public void setEnabled(boolean enabled) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java index 24685f8c805..2e9621b3817 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java @@ -29,7 +29,7 @@ import org.eclipse.jetty.util.log.Logger; public abstract class AbstractLifeCycle implements LifeCycle { private static final Logger LOG = Log.getLogger(AbstractLifeCycle.class); - + public static final String STOPPED="STOPPED"; public static final String FAILED="FAILED"; public static final String STARTING="STARTING"; @@ -196,6 +196,7 @@ public abstract class AbstractLifeCycle implements LifeCycle listener.lifeCycleFailure(this,th); } + @ManagedAttribute(value="The stop timeout in milliseconds") public long getStopTimeout() { return _stopTimeout; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AggregateLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AggregateLifeCycle.java index 1595c607313..f83986bcd08 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AggregateLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AggregateLifeCycle.java @@ -310,6 +310,20 @@ public class AggregateLifeCycle extends AbstractLifeCycle implements Destroyable return false; } + @Override + public void setStopTimeout(long stopTimeout) + { + super.setStopTimeout(stopTimeout); + for (Bean bean : _beans) + { + Object component = bean._bean; + if (component instanceof AbstractLifeCycle) + { + ((AbstractLifeCycle)component).setStopTimeout(stopTimeout); + } + } + } + /** * Dumps to {@link System#err}. * @see #dump() diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index 8928b0192c0..0ba48db0142 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -48,52 +48,33 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo private final AtomicInteger _threadsStarted = new AtomicInteger(); private final AtomicInteger _threadsIdle = new AtomicInteger(); private final AtomicLong _lastShrink = new AtomicLong(); - private final ConcurrentLinkedQueue _threads=new ConcurrentLinkedQueue(); + private final ConcurrentLinkedQueue _threads=new ConcurrentLinkedQueue<>(); private final Object _joinLock = new Object(); private BlockingQueue _jobs; - private String _name; private int _maxIdleTimeMs=60000; - private int _maxThreads; - private int _minThreads; private int _maxQueued=-1; - private int _priority=Thread.NORM_PRIORITY; - private boolean _daemon=false; - private int _maxStopTime=100; - private boolean _detailedDump=false; - /* ------------------------------------------------------------------- */ - /** Construct - */ public QueuedThreadPool() { this(200,8,60000); } - /* ------------------------------------------------------------------- */ - /** Construct - */ public QueuedThreadPool(int maxThreads) { this(maxThreads,8,60000); } - /* ------------------------------------------------------------------- */ - /** Construct - */ public QueuedThreadPool(int maxThreads, int minThreads) { - this(maxThreads,8,60000); + this(maxThreads,minThreads,60000); } - - /* ------------------------------------------------------------------- */ - /** Construct - */ + public QueuedThreadPool(int maxThreads, int minThreads, int maxIdleTimeMs) { _name="qtp"+super.hashCode(); @@ -102,7 +83,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo setMaxIdleTimeMs(maxIdleTimeMs); } - /* ------------------------------------------------------------ */ @Override protected void doStart() throws Exception { @@ -111,7 +91,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo if (_jobs==null) { - _jobs=_maxQueued>0 ?new ArrayBlockingQueue(_maxQueued) + int maxQueued = getMaxQueued(); + _jobs=maxQueued>0 ?new ArrayBlockingQueue(maxQueued) :new BlockingArrayQueue(_minThreads,_minThreads); } @@ -123,7 +104,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } } - /* ------------------------------------------------------------ */ @Override protected void doStop() throws Exception { @@ -131,14 +111,15 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo long start=System.currentTimeMillis(); // let jobs complete naturally for a while - while (_threadsStarted.get()>0 && (System.currentTimeMillis()-start) < (_maxStopTime/2)) + while (_threadsStarted.get()>0 && (System.currentTimeMillis()-start) < (getStopTimeout()/2)) Thread.sleep(1); // kill queued jobs and flush out idle jobs - _jobs.clear(); + BlockingQueue jobs = getQueue(); + jobs.clear(); Runnable noop = new Runnable(){@Override public void run(){}}; for (int i=_threadsIdle.get();i-->0;) - _jobs.offer(noop); + jobs.offer(noop); Thread.yield(); // interrupt remaining threads @@ -147,7 +128,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo thread.interrupt(); // wait for remaining threads to die - while (_threadsStarted.get()>0 && (System.currentTimeMillis()-start) < _maxStopTime) + while (_threadsStarted.get()>0 && (System.currentTimeMillis()-start) < getStopTimeout()) { Thread.sleep(1); } @@ -177,7 +158,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } } - /* ------------------------------------------------------------ */ /** * Delegated to the named or anonymous Pool. */ @@ -186,7 +166,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _daemon=daemon; } - /* ------------------------------------------------------------ */ /** Set the maximum thread idle time. * Threads that are idle for longer than this period may be * stopped. @@ -199,16 +178,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _maxIdleTimeMs=maxIdleTimeMs; } - /* ------------------------------------------------------------ */ - /** - * @param stopTimeMs maximum total time that stop() will wait for threads to die. - */ - public void setMaxStopTimeMs(int stopTimeMs) - { - _maxStopTime = stopTimeMs; - } - - /* ------------------------------------------------------------ */ /** Set the maximum number of threads. * Delegated to the named or anonymous Pool. * @see #getMaxThreads @@ -222,7 +191,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _minThreads=_maxThreads; } - /* ------------------------------------------------------------ */ /** Set the minimum number of threads. * Delegated to the named or anonymous Pool. * @see #getMinThreads @@ -244,7 +212,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } } - /* ------------------------------------------------------------ */ /** * @param name Name of the BoundedThreadPool to use when naming Threads. */ @@ -255,7 +222,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _name= name; } - /* ------------------------------------------------------------ */ /** Set the priority of the pool threads. * @param priority the new thread priority. */ @@ -264,7 +230,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _priority=priority; } - /* ------------------------------------------------------------ */ /** * @return maximum queue size */ @@ -273,7 +238,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _maxQueued; } - /* ------------------------------------------------------------ */ /** * @param max job queue size */ @@ -284,7 +248,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo _maxQueued=max; } - /* ------------------------------------------------------------ */ /** Get the maximum thread idle time. * Delegated to the named or anonymous Pool. * @see #setMaxIdleTimeMs @@ -295,16 +258,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _maxIdleTimeMs; } - /* ------------------------------------------------------------ */ - /** - * @return maximum total time that stop() will wait for threads to die. - */ - public int getMaxStopTimeMs() - { - return _maxStopTime; - } - - /* ------------------------------------------------------------ */ /** Set the maximum number of threads. * Delegated to the named or anonymous Pool. * @see #setMaxThreads @@ -316,7 +269,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _maxThreads; } - /* ------------------------------------------------------------ */ /** Get the minimum number of threads. * Delegated to the named or anonymous Pool. * @see #setMinThreads @@ -329,7 +281,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _minThreads; } - /* ------------------------------------------------------------ */ /** * @return The name of the BoundedThreadPool. */ @@ -339,7 +290,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _name; } - /* ------------------------------------------------------------ */ /** Get the priority of the pool threads. * @return the priority of the pool threads. */ @@ -349,7 +299,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _priority; } - /* ------------------------------------------------------------ */ /** * Delegated to the named or anonymous Pool. */ @@ -359,20 +308,16 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _daemon; } - /* ------------------------------------------------------------ */ - @ManagedAttribute("full stack detail on dump output") public boolean isDetailedDump() { return _detailedDump; } - /* ------------------------------------------------------------ */ public void setDetailedDump(boolean detailedDump) { _detailedDump = detailedDump; } - /* ------------------------------------------------------------ */ @Override public boolean dispatch(Runnable job) { @@ -396,7 +341,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return false; } - /* ------------------------------------------------------------ */ @Override public void execute(Runnable job) { @@ -404,7 +348,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo throw new RejectedExecutionException(toString()); } - /* ------------------------------------------------------------ */ /** * Blocks until the thread pool is {@link LifeCycle#stop stopped}. */ @@ -421,7 +364,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo Thread.sleep(1); } - /* ------------------------------------------------------------ */ /** * @return The total number of threads currently in the pool */ @@ -431,7 +373,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _threadsStarted.get(); } - /* ------------------------------------------------------------ */ /** * @return The number of idle threads in the pool */ @@ -441,7 +382,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _threadsIdle.get(); } - /* ------------------------------------------------------------ */ /** * @return True if the pool is at maxThreads and there are not more idle threads than queued jobs */ @@ -451,7 +391,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _threadsStarted.get()==_maxThreads && _jobs.size()>=_threadsIdle.get(); } - /* ------------------------------------------------------------ */ private boolean startThread(int threads) { final int next=threads+1; @@ -462,8 +401,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo try { Thread thread=newThread(_runnable); - thread.setDaemon(_daemon); - thread.setPriority(_priority); + thread.setDaemon(isDaemon()); + thread.setPriority(getThreadsPriority()); thread.setName(_name+"-"+thread.getId()); _threads.add(thread); @@ -478,14 +417,12 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return started; } - /* ------------------------------------------------------------ */ protected Thread newThread(Runnable runnable) { return new Thread(runnable); } - /* ------------------------------------------------------------ */ @Override @ManagedOperation("dump thread state") public String dump() @@ -493,30 +430,25 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return AggregateLifeCycle.dump(this); } - /* ------------------------------------------------------------ */ @Override public void dump(Appendable out, String indent) throws IOException { - List dump = new ArrayList(getMaxThreads()); + List dump = new ArrayList<>(getMaxThreads()); for (final Thread thread: _threads) { final StackTraceElement[] trace=thread.getStackTrace(); boolean inIdleJobPoll=false; - // trace can be null on early java 6 jvms - if (trace != null) + for (StackTraceElement t : trace) { - for (StackTraceElement t : trace) + if ("idleJobPoll".equals(t.getMethodName())) { - if ("idleJobPoll".equals(t.getMethodName())) - { - inIdleJobPoll = true; - break; - } + inIdleJobPoll = true; + break; } } final boolean idle=inIdleJobPoll; - if (_detailedDump) + if (isDetailedDump()) { dump.add(new Dumpable() { @@ -545,20 +477,17 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo AggregateLifeCycle.dump(out,indent,dump); } - /* ------------------------------------------------------------ */ @Override public String toString() { return String.format("%s{%s,%d<=%d<=%d/%d,%d}",_name,getState(),getMinThreads(),getIdleThreads(),getThreads(),getMaxThreads(),(_jobs==null?-1:_jobs.size())); } - /* ------------------------------------------------------------ */ private Runnable idleJobPoll() throws InterruptedException { return _jobs.poll(_maxIdleTimeMs,TimeUnit.MILLISECONDS); } - /* ------------------------------------------------------------ */ private Runnable _runnable = new Runnable() { @Override @@ -629,7 +558,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } }; - /* ------------------------------------------------------------ */ /** *

Runs the given job in the {@link Thread#currentThread() current thread}.

*

Subclasses may override to perform pre/post actions before/after the job is run.

@@ -641,7 +569,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo job.run(); } - /* ------------------------------------------------------------ */ /** * @return the job queue */ @@ -650,27 +577,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return _jobs; } - /* ------------------------------------------------------------ */ - /** - * @param id The thread ID to stop. - * @return true if the thread was found and stopped. - * @deprecated Use {@link #interruptThread(long)} in preference - */ - @Deprecated - public boolean stopThread(long id) - { - for (Thread thread: _threads) - { - if (thread.getId()==id) - { - thread.stop(); - return true; - } - } - return false; - } - - /* ------------------------------------------------------------ */ /** * @param id The thread ID to interrupt. * @return true if the thread was found and interrupted. @@ -689,7 +595,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo return false; } - /* ------------------------------------------------------------ */ /** * @param id The thread ID to interrupt. * @return true if the thread was found and interrupted. diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java index 68f53549dc2..15c60afc826 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java @@ -4,33 +4,32 @@ // All rights reserved. This program and the accompanying materials // are made available under the terms of the Eclipse Public License v1.0 // and Apache License v2.0 which accompanies this distribution. -// The Eclipse Public License is available at +// The Eclipse Public License is available at // http://www.eclipse.org/legal/epl-v10.html // The Apache License v2.0 is available at // http://www.opensource.org/licenses/apache2.0.php -// You may elect to redistribute this code under either of these licenses. +// You may elect to redistribute this code under either of these licenses. // ======================================================================== package org.eclipse.jetty.util.thread; -import static org.junit.Assert.assertTrue; - import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import junit.framework.Assert; - import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.toolchain.test.annotation.Slow; import org.junit.Test; import org.junit.runner.RunWith; +import static org.junit.Assert.assertTrue; + @RunWith(AdvancedRunner.class) public class QueuedThreadPoolTest { final AtomicInteger _jobs=new AtomicInteger(); - + class RunningJob implements Runnable { private final CountDownLatch _run = new CountDownLatch(1); @@ -38,7 +37,7 @@ public class QueuedThreadPoolTest private final CountDownLatch _stopped = new CountDownLatch(1); public void run() { - try + try { _run.countDown(); _stopping.await(); @@ -53,21 +52,21 @@ public class QueuedThreadPoolTest _stopped.countDown(); } } - + public void stop() throws InterruptedException { if (_run.await(10,TimeUnit.SECONDS)) _stopping.countDown(); if (!_stopped.await(10,TimeUnit.SECONDS)) - throw new IllegalStateException(); + throw new IllegalStateException(); } - }; - - + }; + + @Test @Slow public void testThreadPool() throws Exception - { + { QueuedThreadPool tp= new QueuedThreadPool(); tp.setMinThreads(5); tp.setMaxThreads(10); @@ -75,14 +74,14 @@ public class QueuedThreadPoolTest tp.setThreadsPriority(Thread.NORM_PRIORITY-1); tp.start(); - + waitForThreads(tp,5); waitForIdle(tp,5); - + Thread.sleep(1000); waitForThreads(tp,5); waitForIdle(tp,5); - + RunningJob job=new RunningJob(); tp.dispatch(job); waitForIdle(tp,4); @@ -91,7 +90,7 @@ public class QueuedThreadPoolTest job.stop(); waitForIdle(tp,5); waitForThreads(tp,5); - + Thread.sleep(200); waitForIdle(tp,5); waitForThreads(tp,5); @@ -104,24 +103,24 @@ public class QueuedThreadPoolTest } waitForIdle(tp,0); waitForThreads(tp,5); - + job=new RunningJob(); tp.dispatch(job); waitForThreads(tp,6); - + job.stop(); waitForThreads(tp,5); - + jobs[0].stop(); waitForIdle(tp,1); waitForThreads(tp,5); - + for (int i=1;itrue true true - 1000 + 1000 true false diff --git a/tests/test-integration/src/test/resources/DefaultHandler.xml b/tests/test-integration/src/test/resources/DefaultHandler.xml index 0ef97ffff5b..ce8766feec0 100644 --- a/tests/test-integration/src/test/resources/DefaultHandler.xml +++ b/tests/test-integration/src/test/resources/DefaultHandler.xml @@ -65,6 +65,6 @@ true true true - 1000 + 1000 diff --git a/tests/test-integration/src/test/resources/RFC2616Base.xml b/tests/test-integration/src/test/resources/RFC2616Base.xml index b9da785c11d..6a87fbde4ee 100644 --- a/tests/test-integration/src/test/resources/RFC2616Base.xml +++ b/tests/test-integration/src/test/resources/RFC2616Base.xml @@ -141,6 +141,6 @@ true true true - 1000 + 1000 From c3dfd0c653f788b735c21ed2ceead62f3e77373b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 10 Aug 2012 16:45:05 +0200 Subject: [PATCH 4/4] Jetty9 - Set stopTimeout to zero by default, and setting defaults for threaded component that need a non-zero stop timeout to stop properly. --- .../main/java/org/eclipse/jetty/io/SelectorManager.java | 1 + .../java/org/eclipse/jetty/server/AbstractConnector.java | 9 ++++++++- .../src/main/java/org/eclipse/jetty/server/Server.java | 1 - .../eclipse/jetty/util/component/AbstractLifeCycle.java | 2 +- .../org/eclipse/jetty/util/thread/QueuedThreadPool.java | 3 +++ 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 4ead5b4802f..20d99a051f5 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -268,6 +268,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa public ManagedSelector(int id) { _id = id; + setStopTimeout(5000); } @Override diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java index c2528a74ffb..0ac7570c1ea 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java @@ -15,6 +15,7 @@ package org.eclipse.jetty.server; import java.io.IOException; import java.net.Socket; +import java.nio.channels.AsynchronousCloseException; import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.Executor; @@ -172,7 +173,12 @@ public abstract class AbstractConnector extends AggregateLifeCycle implements Co for (Thread thread : _acceptors) { if (thread != null) + { thread.interrupt(); + long stopTimeout = getStopTimeout(); + if (stopTimeout > 0) + thread.join(stopTimeout); + } } super.doStop(); @@ -273,9 +279,10 @@ public abstract class AbstractConnector extends AggregateLifeCycle implements Co { accept(_acceptor); } - catch (IOException | InterruptedException e) + catch (AsynchronousCloseException | InterruptedException e) { logger.ignore(e); + break; } catch (Throwable e) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 651a54e020f..e3d05b6e80d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -304,7 +304,6 @@ public class Server extends HandlerWrapper implements Attributes MultiException mex=new MultiException(); - // TODO: review this logic ... long stopTimeout = getStopTimeout(); if (stopTimeout>0) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java index 2e9621b3817..6a9ab08a814 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java @@ -41,7 +41,7 @@ public abstract class AbstractLifeCycle implements LifeCycle private final Object _lock = new Object(); private final int __FAILED = -1, __STOPPED = 0, __STARTING = 1, __STARTED = 2, __STOPPING = 3; private volatile int _state = __STOPPED; - private long _stopTimeout = 10000; + private long _stopTimeout = 0; protected void doStart() throws Exception { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index 0ba48db0142..0d578537a7d 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -81,6 +81,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo setMinThreads(minThreads); setMaxThreads(maxThreads); setMaxIdleTimeMs(maxIdleTimeMs); + setStopTimeout(5000); } @Override @@ -110,6 +111,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo super.doStop(); long start=System.currentTimeMillis(); + // TODO: review the stop logic avoiding sleep(1), and eventually using Thread.interrupt() + thread.join() + // let jobs complete naturally for a while while (_threadsStarted.get()>0 && (System.currentTimeMillis()-start) < (getStopTimeout()/2)) Thread.sleep(1);