Application Server 7
  1. Application Server 7
  2. AS7-4675

Memory leak when reading EJB 2.1 CMP EntityBeans

    Details

    • Type: Bug Bug
    • Status: Resolved (View Workflow)
    • Priority: Critical Critical
    • Resolution: Done
    • Affects Version/s: 7.1.1.Final
    • Fix Version/s: 7.1.2.Final (EAP)
    • Component/s: EJB
    • Labels:
      None
    • Environment:
      Windows 7 Pro SP1 (64bit); JDK 1.6.0_31 (64bit); Oracle Database 11g Release 11.1.0.6.0 - 64bit Production; AS71.1Final / AS71.2 latest build
    • Similar Issues:
      Show 10 results 

      Description

      After reading 12000 EJB 2.1 EntityBeans in a test application there are about 5 million Interceptor[] arrays firmely bound in the heap. A heap dump shows this as a typical reference chain:

      GC Root: ServiceContainerImpl$ServiceThread
      ServiceContainerImpl$ServiceThread.container = ServiceContainerImpl
      ServiceContainerImpl.registry = UnlockedReadHashMap
      UnlockedReadHashMap.table = AtomicReferenceArray
      AtomicReferenceArray.array = Object[16384]
      Object[6209] = UnlockedReadHashMap$Item[3]
      UnlockedReadHashMap$Item[1] = UnlockedReadHashMap$Item
      UnlockedReadHashMap$Item.value = ServiceRegistrationImpl
      ServiceRegistrationImpl.instance = ServiceControllerImpl
      ServiceControllerImpl.serviceValue = ImmediateValue
      ImmediateValue.value = TimedObjectInvokerImpl
      TimedObjectInvokerImpl.timeoutInterceptors = HashMap
      HashMap.table = HashMap$Entry[128]
      HashMap$Entry[123] = HashMap$Entry
      HashMap$Entry.value = ChainedInterceptor
      ChainedInterceptor.interceptors = Arrays$ArrayList
      Arrays$ArrayList.a = Interceptor[6]

      The number of bound objects keeps growing when reading more EntityBeans - even wehn reading the same beans again.
      Even after undeploy of the application the

        Gliffy Diagrams

        1. HeapDumpScreenShot.png
          81 kB

          Activity

          Hide
          Stuart Douglas added a comment -

          You are probably actually just running out of memory, rather than this being a leak. 45000 beans is a lot.

          Show
          Stuart Douglas added a comment - You are probably actually just running out of memory, rather than this being a leak. 45000 beans is a lot.
          Hide
          Tiago Frazão added a comment - - edited

          Hi Stuart,

          As Frederico commented,

          When we call a session bean method (tx required) the Jboss 7 must to keep all basic component cached into tx. The point here is : why its been created about 500 hundreds interceptor for each distinct LocalEntity during the invocation ? If you take a look at the picture attached , you will see that 3 major classes as consuming a big porting of our VM (Old G).

          Regard

          Show
          Tiago Frazão added a comment - - edited Hi Stuart, As Frederico commented, When we call a session bean method (tx required) the Jboss 7 must to keep all basic component cached into tx. The point here is : why its been created about 500 hundreds interceptor for each distinct LocalEntity during the invocation ? If you take a look at the picture attached , you will see that 3 major classes as consuming a big porting of our VM (Old G). Regard
          Hide
          Stuart Douglas added a comment -

          Can you give a breakdown of the types of the interceptors (the ones that have the most instances created) ?

          Show
          Stuart Douglas added a comment - Can you give a breakdown of the types of the interceptors (the ones that have the most instances created) ?
          Hide
          Tiago Frazão added a comment -

          Stuart,

          I took a look at the BasicComponent class, which is responsible to create all interceptors for all method in LocalEntity and i suspect the problem is the creation of interceptors for all methods, which I don't use.

          In our code we only call set(s) and gets(), using ValueObject Pattern (setValues(VO) and getValues()), then the idea is to create the interceptor dynamicaly, in other words, the BasicComponent will receive the map of interceptor factories so internally it can create the interceptors when the method is invoked.

          I would like to know if is there any negative impact about this approach ?

          class BasicComponentInstance {
          ...
          //
          // final Map<Method, InterceptorFactory> interceptorFactoryMap = this.getInterceptorFactoryMap();
          // // This is an identity map. This means that only <b>certain</b>

          {@code Method}

          objects will
          // // match - specifically, they must equal the objects provided to the proxy.
          // final IdentityHashMap<Method, Interceptor> interceptorMap = new IdentityHashMap<Method, Interceptor>();
          // for (Method method : interceptorFactoryMap.keySet())

          { // interceptorMap.put(method, interceptorFactoryMap.get(method).create(context)); // }

          ...

          // create the component instance
          final BasicComponentInstance basicComponentInstance =
          this.instantiateComponentInstance(instanceReference,
          componentInstancePreDestroyInterceptor,
          this.getInterceptorFactoryMap(), // ---> We pass the factories not the interceptors instances
          context);

          }

          class BasicComponentInstance {
          protected BasicComponentInstance(final BasicComponent component,
          final AtomicReference<ManagedReference> instanceReference,
          final Interceptor preDestroyInterceptor,
          final Map<Method, InterceptorFactory> interceptorFactoryMap,
          final InterceptorFactoryContext context // --> We are passing the context use by the factory
          )

          { // Associated component this.component = component; this.instanceReference = instanceReference; this.preDestroy = preDestroyInterceptor; this.interceptorFactoryMap = interceptorFactoryMap; this.context = context; }

          public Interceptor getInterceptor(final Method method) throws IllegalStateException {
          //Interceptor interceptor = methodMap.get(method);
          // Here we will get the interceptor factory for the method trigged. (on demand)

          InterceptorFactory interceptorFactory = this.interceptorFactoryMap.get(method);

          if (!this.methodMap.containsKey(method))

          { this.methodMap.put(method, interceptorFactory.create(this.context)); }

          Interceptor interceptor = this.methodMap.get(method);
          if (interceptor == null)

          { throw MESSAGES.methodNotFound(method); }

          return interceptor;
          }

          }

          Show
          Tiago Frazão added a comment - Stuart, I took a look at the BasicComponent class, which is responsible to create all interceptors for all method in LocalEntity and i suspect the problem is the creation of interceptors for all methods, which I don't use. In our code we only call set(s) and gets(), using ValueObject Pattern (setValues(VO) and getValues()), then the idea is to create the interceptor dynamicaly, in other words, the BasicComponent will receive the map of interceptor factories so internally it can create the interceptors when the method is invoked. I would like to know if is there any negative impact about this approach ? class BasicComponentInstance { ... // // final Map<Method, InterceptorFactory> interceptorFactoryMap = this.getInterceptorFactoryMap(); // // This is an identity map. This means that only <b>certain</b> {@code Method} objects will // // match - specifically, they must equal the objects provided to the proxy. // final IdentityHashMap<Method, Interceptor> interceptorMap = new IdentityHashMap<Method, Interceptor>(); // for (Method method : interceptorFactoryMap.keySet()) { // interceptorMap.put(method, interceptorFactoryMap.get(method).create(context)); // } ... // create the component instance final BasicComponentInstance basicComponentInstance = this.instantiateComponentInstance(instanceReference, componentInstancePreDestroyInterceptor, this.getInterceptorFactoryMap(), // ---> We pass the factories not the interceptors instances context); } class BasicComponentInstance { protected BasicComponentInstance(final BasicComponent component, final AtomicReference<ManagedReference> instanceReference, final Interceptor preDestroyInterceptor, final Map<Method, InterceptorFactory> interceptorFactoryMap, final InterceptorFactoryContext context // --> We are passing the context use by the factory ) { // Associated component this.component = component; this.instanceReference = instanceReference; this.preDestroy = preDestroyInterceptor; this.interceptorFactoryMap = interceptorFactoryMap; this.context = context; } public Interceptor getInterceptor(final Method method) throws IllegalStateException { //Interceptor interceptor = methodMap.get(method); // Here we will get the interceptor factory for the method trigged. (on demand) InterceptorFactory interceptorFactory = this.interceptorFactoryMap.get(method); if (!this.methodMap.containsKey(method)) { this.methodMap.put(method, interceptorFactory.create(this.context)); } Interceptor interceptor = this.methodMap.get(method); if (interceptor == null) { throw MESSAGES.methodNotFound(method); } return interceptor; } }
          Hide
          Frederico Francisco added a comment -

          Stuart Douglas,

          Actually the following was removed from ee/src/main/java/org/jboss/as/ee/component/BasicComponent.java:

          final Map<Method, InterceptorFactory> interceptorFactoryMap = this.getInterceptorFactoryMap();
          // This is an identity map. This means that only <b>certain</b>

          {@code Method}

          objects will
          // match - specifically, they must equal the objects provided to the proxy.
          final IdentityHashMap<Method, Interceptor> interceptorMap = new IdentityHashMap<Method, Interceptor>();
          for (Method method : interceptorFactoryMap.keySet())

          { interceptorMap.put(method, interceptorFactoryMap.get(method).create(context)); }

          Instead of creating the identiy map above, the interceptorFactoryMap is passed to the constructor of BasicComponentInstance, which internally will populate the interceptorMap, on demand, as methods are called.

          All the subclasses were changed to receive the interceptorFactoryMap and the InterceptorContext in their constructor.

          Could you give us some feedback? Are these changes sane? Are we missing something?

          Thanks.

          Show
          Frederico Francisco added a comment - Stuart Douglas, Actually the following was removed from ee/src/main/java/org/jboss/as/ee/component/BasicComponent.java: final Map<Method, InterceptorFactory> interceptorFactoryMap = this.getInterceptorFactoryMap(); // This is an identity map. This means that only <b>certain</b> {@code Method} objects will // match - specifically, they must equal the objects provided to the proxy. final IdentityHashMap<Method, Interceptor> interceptorMap = new IdentityHashMap<Method, Interceptor>(); for (Method method : interceptorFactoryMap.keySet()) { interceptorMap.put(method, interceptorFactoryMap.get(method).create(context)); } Instead of creating the identiy map above, the interceptorFactoryMap is passed to the constructor of BasicComponentInstance, which internally will populate the interceptorMap, on demand, as methods are called. All the subclasses were changed to receive the interceptorFactoryMap and the InterceptorContext in their constructor. Could you give us some feedback? Are these changes sane? Are we missing something? Thanks.

            People

            • Assignee:
              Stuart Douglas
              Reporter:
              Klaus Benary
            • Votes:
              2 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development