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 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

      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: