-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
In the FullStackTest
, the EntityManagerProvider
incorrectly relies on disposal method
#376
Comments
I would add that not closing the entitymanager can cause issues that are hard to debug. For example, a similar issue happens in Quarkus when using If possible, it would be nice to warn users if we already know that the dispose method won't get called. |
The Quarkus issue is, in essence, different. There, the disposal should be called but it was handled incorrectly from what I can tell. In Weld, this is currently a limitation and a hard one to bypass at that (mainly because of support for session/conversation). We could maybe add some INFO one-time logging into the CDI context provider in this project to let people know? |
Yes, I mentioned the issue to highlight the impact of not calling the dispose. And the importance of letting users know about these situations in advance. |
How do you detect that? |
What I meant is to simply LOG an info that CP in combination with Weld is used and that users shouldn't rely on predestroy/dispose methods. This could be done in[ |
This was originally brought up in #375
The provider in question is [this class](https://github.com/smallrye/smallrye-context-propagation/blob/main/tests/src/main/java/io/smallrye/context/test/EntityManagerProvider.java#L29-L32.
This logic won't work if there is CP involved and Weld is used. It is documented in this part of Weld docs but in short, the way we propagate in
CDIContextProvider
is that we only ever deactivate contexts, but never invalidate them so all pre destroy and disposer methods are skipped. This is because otherwise we would invoke them multiple times, potentially leading too unexpected states.For what we test, this doesn't matter but it is showing a state that doesn't work and in real app, the EM would never close.
Therefore, we should:
em.close()
logic elsewhere. Not sure where, maybe@BeforeDestroyed(RequestScoped.class)
and we should check if the bean exists before doing that.The text was updated successfully, but these errors were encountered: