4 Replies Latest reply on Dec 21, 2005 10:06 PM by Clebert Suconic

    JBossAS-2238: BasicMBeanRegistry synchronization

    Adrian Brock Master

      http://jira.jboss.com/jira/browse/JBAS-2238

      I would guess that somebody has broken the synchronization. Although I can't
      see an obvious recent change.

      This has been confusing in this class, ever since the jmx-remoting team reworked the synchronization.
      Sometimes it synchronizes on the BasicMBeanRegistry instance,
      sometimes on DomainMap, sometimes on both.

      I propose the best approach is to convert the domain map and its submaps
      to use a ConcurrentReaderHashMap. This would remove most of the need for
      synchronization, except for the add/remove critical sections.

        • 2. Re: JBossAS-2238: BasicMBeanRegistry synchronization
          Clebert Suconic Master

          The current implementation of BasicMBeanRegistry is causing locking contention:

          Example:

          "http-172.16.133.197-8080-716" daemon prio=10 tid=0x01788e60 nid=0x4fe waiting for monitor entry [0x3e1fe000..0x3e1ffc90]
           at org.jboss.mx.server.registry.BasicMBeanRegistry.getMBeanMap(BasicMBeanRegistry.java:959)
           - waiting to lock <0x8f6867f0> (a EDU.oswego.cs.dl.util.concurrent.ConcurrentReaderHashMap)
           at org.jboss.mx.server.registry.BasicMBeanRegistry.get(BasicMBeanRegistry.java:518)
           at org.jboss.mx.server.MBeanServerImpl.invoke(MBeanServerImpl.java:653)
           at org.jboss.invocation.local.LocalInvoker$MBeanServerAction.invoke(LocalInvoker.java:169)
           at org.jboss.invocation.local.LocalInvoker.invoke(LocalInvoker.java:118)
           at org.jboss.invocation.InvokerInterceptor.invokeLocalMarshalled(InvokerInterceptor.java:296)
           at org.jboss.invocation.MarshallingInvokerInterceptor.invoke(MarshallingInvokerInterceptor.java:61)
           at org.jboss.proxy.TransactionInterceptor.invoke(TransactionInterceptor.java:61)
           at org.jboss.proxy.SecurityInterceptor.invoke(SecurityInterceptor.java:70)
           at org.jboss.proxy.ejb.StatefulSessionInterceptor.invoke(StatefulSessionInterceptor.java:121)
           at org.jboss.proxy.ClientContainer.invoke(ClientContainer.java:100)
          



          I would like to change BasicMBeanRegistry to be:

          
           private Map getMBeanMap(String domain, boolean createIfMissing)
           {
           Map mbeanMap = (Map) domainMap.get(domain);
           if (mbeanMap == null && createIfMissing)
           {
           synchronized (domainMap)
           {
           mbeanMap = (Map) domainMap.get(domain);
           if (mbeanMap == null && createIfMissing)
           {
           mbeanMap = new ConcurrentReaderHashMap();
           domainMap.put(domain, mbeanMap);
           } // end of if ()
           }
           }
           return mbeanMap;
           }
          
          


          If there is an issue with that change, what would be another suggestion for this?


          • 3. Re: JBossAS-2238: BasicMBeanRegistry synchronization
            Scott Stark Master

            I don't see that the synchronization on the domainMap is needed at all. The add(MBeanEntry entry) and remove(ObjectName name) methods that modify the domainMap contents are already synchronized. The getMBeanMap comment needs to be updated and should state that calling this method with createIfMissing needs to be done from a synchronized method.