From e09396db4ef5800160a77cb0f08388a140e2432a Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 19 Feb 2016 16:32:44 +0100 Subject: [PATCH 1/3] Issue #298 (qtp threads spin-locked in MBeanContainer.beanAdded). Replaced WeakHashMap with ConcurrentMap. The "weak" features of WHM were not used anyway. --- .../org/eclipse/jetty/jmx/MBeanContainer.java | 193 ++++++++---------- 1 file changed, 86 insertions(+), 107 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 998b45b99ab..dfe95fcf0e5 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 @@ -21,7 +21,6 @@ package org.eclipse.jetty.jmx; import java.io.IOException; import java.util.Locale; import java.util.Map; -import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; @@ -29,7 +28,6 @@ import java.util.concurrent.atomic.AtomicInteger; import javax.management.InstanceNotFoundException; import javax.management.MBeanRegistrationException; import javax.management.MBeanServer; -import javax.management.ObjectInstance; import javax.management.ObjectName; import org.eclipse.jetty.util.component.Container; @@ -37,7 +35,6 @@ import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker; /** * Container class for the MBean instances @@ -45,16 +42,15 @@ import org.eclipse.jetty.util.thread.Locker; public class MBeanContainer implements Container.InheritedListener, Dumpable { private final static Logger LOG = Log.getLogger(MBeanContainer.class.getName()); - private final static ConcurrentMap __unique = new ConcurrentHashMap(); + private final static ConcurrentMap __unique = new ConcurrentHashMap<>(); public static void resetUnique() { __unique.clear(); } - - private final Locker _lock = new Locker(); + private final MBeanServer _mbeanServer; - private final WeakHashMap _beans = new WeakHashMap(); + private final Map _beans = new ConcurrentHashMap<>(); private String _domain = null; /** @@ -63,31 +59,23 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable * @param object instance for which object name is looked up * @return object name associated with specified instance, or null if not found */ - public synchronized ObjectName findMBean(Object object) + public ObjectName findMBean(Object object) { - try (Locker.Lock lock = _lock.lock()) - { - ObjectName bean = _beans.get(object); - return bean == null ? null : bean; - } + return _beans.get(object); } /** * Lookup an instance by object name * - * @param oname object name of instance + * @param objectName object name of instance * @return instance associated with specified object name, or null if not found */ - public synchronized Object findBean(ObjectName oname) + public Object findBean(ObjectName objectName) { - try (Locker.Lock lock = _lock.lock()) + for (Map.Entry entry : _beans.entrySet()) { - for (Map.Entry entry : _beans.entrySet()) - { - ObjectName bean = entry.getValue(); - if (bean.equals(oname)) - return entry.getKey(); - } + if (entry.getKey().equals(objectName)) + return entry.getValue(); } return null; } @@ -137,43 +125,43 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable public void beanAdded(Container parent, Object obj) { if (LOG.isDebugEnabled()) - LOG.debug("beanAdded {}->{}",parent,obj); + LOG.debug("beanAdded {}->{}", parent, obj); - try (Locker.Lock lock = _lock.lock()) + // Is there an object name for the parent ? + ObjectName parentObjectName = null; + if (parent != null) { - // Is their an object name for the parent - ObjectName pname=null; - if (parent!=null) + parentObjectName = findMBean(parent); + if (parentObjectName == null) { - pname=_beans.get(parent); - if (pname==null) - { - // create the parent bean - beanAdded(null,parent); - pname=_beans.get(parent); - } + // Create the parent bean. + beanAdded(null, parent); + parentObjectName = findMBean(parent); } + } - // Does an mbean already exist? - if (obj == null || _beans.containsKey(obj)) - return; + // Does the mbean already exist ? + if (obj == null || _beans.containsKey(obj)) + return; - // Create an MBean for the object + try + { + // Create an MBean for the object. Object mbean = ObjectMBean.mbeanFor(obj); if (mbean == null) return; - ObjectName oname = null; + ObjectName objectName = null; if (mbean instanceof ObjectMBean) { ((ObjectMBean)mbean).setMBeanContainer(this); - oname = ((ObjectMBean)mbean).getObjectName(); + objectName = ((ObjectMBean)mbean).getObjectName(); } - //no override mbean object name, so make a generic one - if (oname == null) - { - //if no explicit domain, create one + // No override of the mbean's ObjectName, so make a generic one. + if (objectName == null) + { + // If no explicit domain, create one. String domain = _domain; if (domain == null) domain = obj.getClass().getPackage().getName(); @@ -183,43 +171,44 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable if (dot >= 0) type = type.substring(dot + 1); + StringBuilder buf = new StringBuilder(); - StringBuffer buf = new StringBuffer(); + String context = (mbean instanceof ObjectMBean) ? makeName(((ObjectMBean)mbean).getObjectContextBasis()) : null; + if (context == null && parentObjectName != null) + context = parentObjectName.getKeyProperty("context"); - String context = (mbean instanceof ObjectMBean)?makeName(((ObjectMBean)mbean).getObjectContextBasis()):null; - if (context==null && pname!=null) - context=pname.getKeyProperty("context"); - - if (context != null && context.length()>1) + if (context != null && context.length() > 1) buf.append("context=").append(context).append(","); buf.append("type=").append(type); - String name = (mbean instanceof ObjectMBean)?makeName(((ObjectMBean)mbean).getObjectNameBasis()):context; - if (name != null && name.length()>1) + String name = (mbean instanceof ObjectMBean) ? makeName(((ObjectMBean)mbean).getObjectNameBasis()) : context; + if (name != null && name.length() > 1) buf.append(",").append("name=").append(name); String basis = buf.toString(); AtomicInteger count = __unique.get(basis); - if (count==null) + if (count == null) { - count=__unique.putIfAbsent(basis,new AtomicInteger()); - if (count==null) - count=__unique.get(basis); + count = new AtomicInteger(); + AtomicInteger existing = __unique.putIfAbsent(basis, count); + if (existing != null) + count = existing; } - oname = ObjectName.getInstance(domain + ":" + basis + ",id=" + count.getAndIncrement()); + objectName = ObjectName.getInstance(domain + ":" + basis + ",id=" + count.getAndIncrement()); } - ObjectInstance oinstance = _mbeanServer.registerMBean(mbean, oname); + _mbeanServer.registerMBean(mbean, objectName); if (LOG.isDebugEnabled()) - LOG.debug("Registered {}", oinstance.getObjectName()); - _beans.put(obj, oinstance.getObjectName()); + LOG.debug("Registered {}", objectName); + + _beans.put(obj, objectName); } - catch (Exception e) + catch (Throwable x) { - LOG.warn("bean: " + obj, e); + LOG.warn("bean: " + obj, x); } } @@ -227,29 +216,12 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable public void beanRemoved(Container parent, Object obj) { if (LOG.isDebugEnabled()) - LOG.debug("beanRemoved {}",obj); - try (Locker.Lock lock = _lock.lock()) - { - ObjectName bean = _beans.remove(obj); + LOG.debug("beanRemoved {}", obj); - if (bean != null) - { - try - { - _mbeanServer.unregisterMBean(bean); - if (LOG.isDebugEnabled()) - LOG.debug("Unregistered {}", bean); - } - catch (javax.management.InstanceNotFoundException e) - { - LOG.ignore(e); - } - catch (Exception e) - { - LOG.warn(e); - } - } - } + ObjectName objectName = _beans.remove(obj); + + if (objectName != null) + unregister(objectName); } /** @@ -258,19 +230,22 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable */ public String makeName(String basis) { - if (basis==null) - return basis; - return basis.replace(':', '_').replace('*', '_').replace('?', '_').replace('=', '_').replace(',', '_').replace(' ', '_'); + if (basis == null) + return null; + return basis + .replace(':', '_') + .replace('*', '_') + .replace('?', '_') + .replace('=', '_') + .replace(',', '_') + .replace(' ', '_'); } @Override public void dump(Appendable out, String indent) throws IOException { - try (Locker.Lock lock = _lock.lock()) - { - ContainerLifeCycle.dumpObject(out,this); - ContainerLifeCycle.dump(out, indent, _beans.entrySet()); - } + ContainerLifeCycle.dumpObject(out,this); + ContainerLifeCycle.dump(out, indent, _beans.entrySet()); } @Override @@ -281,22 +256,26 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable public void destroy() { - try (Locker.Lock lock = _lock.lock()) + _beans.values().stream() + .filter(objectName -> objectName != null) + .forEach(this::unregister); + } + + private void unregister(ObjectName objectName) + { + try { - for (ObjectName oname : _beans.values()) - { - if (oname!=null) - { - try - { - _mbeanServer.unregisterMBean(oname); - } - catch (MBeanRegistrationException | InstanceNotFoundException e) - { - LOG.warn(e); - } - } - } + getMBeanServer().unregisterMBean(objectName); + if (LOG.isDebugEnabled()) + LOG.debug("Unregistered {}", objectName); + } + catch (MBeanRegistrationException | InstanceNotFoundException x) + { + LOG.ignore(x); + } + catch (Throwable x) + { + LOG.warn(x); } } } From 2e62bca2ea5de4c01c6b0797a36093f7763f1c67 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 19 Feb 2016 16:33:55 +0100 Subject: [PATCH 2/3] Issue #251 (Consider removing SSLEngine.beginHandshake() calls). Removed SSLEngine.beginHandshake() calls. --- .../org/eclipse/jetty/io/ssl/SslConnection.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 50cfa922d97..cf2a615eff0 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -35,7 +35,6 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.EofException; -import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.io.SelectChannelEndPoint; import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.util.BufferUtil; @@ -143,18 +142,8 @@ public class SslConnection extends AbstractConnection @Override public void onOpen() { - try - { - // Begin the handshake - _sslEngine.beginHandshake(); - super.onOpen(); - getDecryptedEndPoint().getConnection().onOpen(); - } - catch (SSLException x) - { - getEndPoint().close(); - throw new RuntimeIOException(x); - } + super.onOpen(); + getDecryptedEndPoint().getConnection().onOpen(); } @Override From a57f8d05172d05326963a412d494184983bf1874 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 19 Feb 2016 16:51:23 +0100 Subject: [PATCH 3/3] Updated the certificate to make the test pass with the latest JDKs. --- .../src/test/config/etc/jetty-ssl.xml | 1 - .../src/test/config/etc/keystore | Bin 1416 -> 2206 bytes 2 files changed, 1 deletion(-) diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-ssl.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-ssl.xml index f82386e1e78..dce437de0f8 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-ssl.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-ssl.xml @@ -33,7 +33,6 @@ / - / diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/keystore b/jetty-osgi/test-jetty-osgi/src/test/config/etc/keystore index 08f6cda8a7b0104236c37f3c06b4cda1d8fc58e6..428ba54776ede2fdcdeedd879edb927c2abd9953 100644 GIT binary patch literal 2206 zcmcgt`9Bkm8{cNkoMUp6gmShKn!AQX*(l6Nj(i=TnQPOKYtv{*Wg>ItE=Q!pRYH8a z$Sp#S#2lYw#aw;$y9u4T}83H*%lp zAKZay0sy=q1Qoo85aAQh;$ zD(c2EIN#D7WwYDLKUg!CotQPD@dp;5FR#bgaace(^x$6g5frD~(_b(MI^J&*A2DRp zf5Q2onfE(zvUb9|9C`66)YFRNM6~xrz4;iVbU=P|*YT2eWHFJJtr+M@zt2qPm)K~rRcqcs=LM12)PX0TT%QO zlf*xkqD3}7l)1J`5W(>=9nR0e6j-<79<11v3ZuXXcQpoCsqY~n`$FN+S}hcVm5Y>G zXnD{@DYs1@{S0z(lW+?86LWKtku$$-(khsh>0qRUXn=84`GRn?77M^_JY`durnN;KE zW#OJ`h<6xcB{I))ekGpc*Ylt}0cx4|OMBDPQvx4`r`}4Ze5_ipdObGMTi3bZHd5PC zcY0;?uBWu$PSvjJeb87nY7ghNv?%M@SoDl6IWt`bQCosfSh$#D6$ea~QhKM^ud2Ut z+9PYJuVpoELmN-A`F$BicO{BSYg@#tS%avVfb}DxL)|NanJ)#zB!2~?#Ot%H7--9N zU$bs0fS5G!m5M4&WK3#a|H|Tgw*?X-;H+Lu@kwA>qSR~7UC7b)7MJXTn6PG>n@8jP zW+}F^X$$c;U~4ryqRF; z>`j!tbLMK4ZGyY643|~?%Mu#fm!l%wAKjBDmd+VYmp3S#$scD$~bxbf|z#)hShN0*AhRaPDcmqrftGlHq4^54MM$Xfy(2> zH8QYVMzmn_oHbvJCB`IN~E&{1*h&0gEM{e zKvWvzp(!BqMX8`t#)~0nq}Wa zr6>FRPyp;AAB&)1$5@;r$23J{K&~>TWjZf7V$wFzmGM95CXhFG1cJNVAXks}C+&2- zbf9Qn*D8N}Afd2kpwDxns3%1uaFhAqDV8ksWiWY|quuLGZ0)SqrJ!Y8yX}@}IyC$C zQ3rCUsn}#>F#D8%D?q~ySy4j&he%Bs{{7V%rl!ui`@KQP?NTi+_iN{cwom&9RaMRR zB~z!hz|0HAgB9_Ijvpe-zr#jLbckJsc>vmo{+im?t8lA;N#fD4?{lb&J0V8Gocq%; f1ihv=QIDh{M_<9V+45Z2{KE4_qW}V3B0uV%GgrOJ literal 1416 zcmezO_TO6u1_mY|W&~r_tkjZ{N+3_R)UekNZbhDKH`EXUR-=hO7hY&y{H-e(+TowuJ*{`? zwVfT`ns@KmCLd9_H?QZ%#(VNFq-*~l&ib%)(Y!;gI{W`RJIFow^~EB{E~hHzXm8Qd z$+I3OFWL4l%D`0pEH}^DdFqAf_3vNy-07jRmW@8^t|?HpH{w+9zJZ0HTQ{GwgE`KsL0)*BinM>ExK{8V|s z_c83?=Ue&W%>UG?wT~>g^C-D5b6Kid$utMyOUY~8&RQ2O5Kv$6WA72RZCZOI`%XW7 zWGv*iET~D-A?Uf$)PkAXC#^&coOKW1)_jbSp1ybc*-Pg z*1AKX?hJ-g?<}Z|$~fj~%&Q)8GVXHmuY_w7iL*9Li<#ba=>(4shvkf&zalFllPfRmB({Gr>G+(AG{|j5^yi-$J^G%gy&!3%F`Y=?Z zqHag3(zm<4?6(-6S8caBa7ykAzsvLfb>|ds{`&FW?ULoW&INs`nHskiv_4^yUS(;u zwJG}=M=euvt`d*!{pAW3tsAR0lWEH+#d4(r%9_{jEh_BIde_H z2F(udYbrB!9Sz@xPFu0!#>2SI`Ssat!p3^(;?40Uy|?vOtS!3O8Zd1|F}L60tKCOm znylYEA@-w}ri%BqU%N_+XGl&zI@4*vv5nXG(`N27=6zy$WG5s+N9dUvSOODr2QVSG z7&I}yWn%FZKK#XimyJ`a&7GmI50%m z5h5xEN+4Za!qUF^MI{POiIob@`FX{qIVG8S=?VcQl?py3DTaIoJRnuv!mM7P3}z^0 zAOYet3k!lXoL(+aZ&G5VUVc%!ft)z6frX)=fw_T+k%h5YlsK=knSrs9DU>^yoZ2`a zIUIo{19M|9Ff2Qn8XFm!A3o&OeJrUOYjfQ^)giy{^|QK1CEP2QH%vD1Nc3MB^xmW4 z2fxzex2hhF*O)r=%WpirQ+H{vy7qk+o=56U7bdiMykRRo{_oVH)zDhW2yL+!vyn`wiA33|lR42Pgou}g~dvauF+U45K+=Wccj0}v( z&I3j>GtgZkrPhJg=H9<%l_$GC*-}{lrfI>Hv$DOvyPj6+9TS$eJZ37ao71_#A!EVD zCY$?z=d9ULtT&}=o8QIxpMTDAp0ptI@ccyM2W9WgVozM<*}KLmGg5O$WNJ zyVS~6#D7};!ib^4U#WeU%`4Hj5}6$f=N+nWnZl~&!Rl~+N3e