kartiksirigeri commented on PR #2408: URL: https://github.com/apache/tomee/pull/2408#issuecomment-3750340369
> Hi All, > > First, @kartiksirigeri thank you so much for taking the initiative to try and fix this. It's a big code base and you dug right in. That says a lot of very good things about you. > > I can verify this is a memory leak. Some notes first. > > It's not legal to put `@Remove` on the interface as this annotation is usable on the bean class only. We do have validation to check for this common mistake and the test case does have a warning stating the annotation was ignored: > > ``` > Jan 12, 2026 3:23:23 PM org.apache.openejb.util.LogStreamAsync run > WARNING: WARN ... MyBean: Ignoring @Remove used on interface org.apache.openejb.core.stateful.StatefulBeanRegistryCleanupTest$MyBeanInterface method cleanup. Annotation only usable on the bean class. > Jan 12, 2026 3:23:23 PM org.apache.openejb.util.LogStreamAsync run > WARNING: WARN ... MyBean2: Ignoring @Remove used on interface org.apache.openejb.core.stateful.StatefulBeanRegistryCleanupTest$MyBeanInterface2 method cleanup. Annotation only usable on the bean class. > ``` > > The upshot of this is that the fix checks the interface for `@Remove` and is not valid. It also causes a new leak in the scenario where the interface was illegally annotated `@Remove` and the bean class has no `@Remove` method. The result is that the handler is invalidated while the actual stateful instance stays living. The container sees `cleanup()` as a regular business method and simply invokes it and does not remove the bean instance. > > This all said, the handler leak discovered is still very valid. If you were to move the `@Remove` annotation to the `cleanup()` method defined in the bean class, the handler reference in the registry would still not get cleaned up when it's called. > > I dug through the code looking for a way to get the knowledge that `cleanup` is a remove method somehow communicated to the Handler, but do not see any good way to do that. The actual mapping of what methods are remove methods is inside private fields accessible to the StatefulContainer only. That is by design. > > I dug through the code trying to refresh my memory on why we even have a registry in the first place and came to the conclusion it might be something we could potentially remove entirely. We'd only be able to do that on the TomEE 11 branch and before it goes final. > > @kartiksirigeri if that's something you might be interested taking the lead on, join the dev list ([mailto:[email protected]](mailto:[email protected])) and we can talk more there. Hi @dblevins, i had some doubt too on where the annotation should be added while making this change, however the changes made at least gave a confirmation to me that the real cause was the registry. I do not understand the specs/codebase much but would be happy to contribute... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
