fix for TOMEE-4560

[kartiksirigeri]

Proxy handler does not remove the stateful EJB3 from the liveregistry on invocation of @remove annotated method. Check is added to identify invocation of @remove annotated method of stateful EJB3.
Defect link : https://issues.apache.org/jira/browse/TOMEE-4560

[kartiksirigeri]

the build for the pull request failed with, 'Error: This request has been automatically failed because it uses a deprecated version of actions/cache: v2. Please update your workflow to use v3/v4 of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-12-05-notice-of-upcoming-releases-and-breaking-changes-for-github-actions/#actions-cache-v1-v2-and-actions-toolkit-cache-package-closing-down'...

I do not understand this error.

[rzo1]

Thanks for the PR. Please note: https://tomee.apache.org/tomee-8.0-eol.html

The branch uses an EOL GitHub Action version. For that reason the build fails right now.

[kartiksirigeri]

Thanks for the PR. Please note: https://tomee.apache.org/tomee-8.0-eol.html

The branch uses an EOL GitHub Action version. For that reason the build fails right now.

https://tomee.apache.org/tomee-8.0-eol.html says 'bugs affecting only the tomee-8.x branch will not be addressed' , I am not sure if the issue is only for 8.x as I have been using only this version

[rzo1]

Yes, it is very likely that this is also present in newer TomEE versions, as the class itself did not change apart from the javax → jakarta migration. What I meant with the eol notice is that it’s unlikely we’ll do another TomEE 8 release. However, since the issue appears to affect newer TomEE versions as well, we can address it there.

Could we add a unit test to verify the change and reproduce the issue in the first place? From what I can see in the Jira ticket, reproduction code was already provided, so it should be possible to turn that into a unit test.

[kartiksirigeri]

provided

managed to add a unit test case, hope that helps, this test should fail now without the fix...

[dblevins]

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:dev-subscribe@tomee.apache.org) and we can talk more there.

[kartiksirigeri]

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:dev-subscribe@tomee.apache.org) 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...

[rzo1]

I do not understand the specs/codebase much but would be happy to contribute...

@kartiksirigeri Great to hear! Please feel free to join the dev mailing list (mailto:dev-subscribe@tomee.apache.org) and start a new thread about the registry. We truly welcome all contributions - so thank you for taking the initiative and opening this PR.

There has been some prior discussion on the topic here, but I agree that continuing the conversation in a new thread makes sense. In short (to summarize @dblevins from the other thread), we would need to (at least attempt to) run the EJB TCK of EE 9.1. Happy to share more details on the mailing list.