Intermittent NullPointerException when use hibernate with context propagation #11295
Comments
quarkusbot
added
area/hibernate-orm
|
Hi, thanks for reporting! from checking the ORM code, it looks like that to trigger this specific NPE we'd need a concurrent invocation on Any suggestion @mkouba , @FroMage or @stuartwdouglas ? Could we do better, or at least detect and block using such a pattern? |
|
I've pushed this change to Hibernate ORM: Wondering if Quarkus would be able to preven this in a better way. |
|
@Sanne I am actually a bit surprised that this usage scenario is not supported? |
The problem I see in the code above is that the EntityManager is being created in one thread (the one requesting the injection) and then that one single EntityManager is passed to N threads to be used. The EntityManager is not threadsafe.
I'm not familiar with context propagation (so perhaps there's a better way to do this) but the way I'd do this myself is to pass the EntityManagerFactory to other threads, and then explicitly open/close a new EntityManager within each "task" scope. In particular when writing code which does explicit threading I'd tend to favour being all-in explicit with scope boundaries of the EntityManager lifecycle, its fllush and load cycles, and transaction boundaries. Alternatively, if simpicity is more important, refrain from explicit threading. |
|
Ok, I can see it now: All So either each |
|
Exactly, @famod ; this is a violation of the ORM contracts. We could close the issue, but I'm not sure if "context propagation" is in some way suggesting that this should be doable. And ideally I'd hope we could detect such cases and provide a better error message - I've included a small improvement in ORM but I don't think it's effective enough, as it will only trigger in rare conditions (and probably too late to prevent trouble). |
|
No, CP exists for the use-cases of propagating contexts that work on multiple threads provided that:
You can exclude contexts by creating your own |
Could you clarify what you're saying "No" to @FroMage ? You don't agree we should close this issue? |
|
I was replying to your question "I'm not sure if "context propagation" is in some way suggesting that this should be doable." No, it should not be doable, so yes you can close this, though it'd be nice to be able to detect this. |
|
Can you think of a way to detect and prevent such a mistake? This seems very important to me, but I'm not familiar at all with the context propagation design. |
|
I can't think of any way to detect and prevent this, precisely because it's alright to access a transaction from multiple threads, as long as it's not "at the same time", so with proper user synchronisation. Given that it's the ORM operations that are problematic, we could try to detect that an operation is ongoing, either by synchronizing them or keeping track of ongoing calls, but that's some work, and it will be costly even for people who don't try this fancy kind of death trap. We could look at which thread is currently invoking the ORM operation, like we do for preventing it on the IO thread, but it would only tell us that we're in a worker thread, and the So, sorry, I have several ideas of what would not work, but no idea as to what could work. |
|
ok, thanks @FroMage - I appreciate the thoughts. From that I'd conclude:
For the record point 1 bothers me, as I'd expect most code to fall into this cathegory - especially end user's code. cc/ @gavinking |
yrodiere
added
area/context-propagation
and removed
area/persistence
I changed the labels of this issue to reflect this. |
|
I was unable to reproduce it anymore. Closing this issue - please re-open if it's still relevant (with an updated reproducer) |
cescoffier
added
the
triage/out-of-date
Describe the bug
Using hibernate and context propagation with
ManagedExecutoroccur an intermittent NullPointerException. I runs this code and sometimes it runs with success but other time it fail.Expected behavior
No throw NullPointerException
Actual behavior
To Reproduce
Steps to reproduce the behavior:
BarbeansConfiguration
# Add your application.properties here, if applicable.Screenshots
(If applicable, add screenshots to help explain your problem.)
Environment (please complete the following information):
uname -aorver: Linux 5.4.0-42-generic Add proper logging #46-Ubuntu SMP Fri Jul 10 00:24:02 UTC 2020 x86_64 x86_64 x86_64 GNU/Linuxjava -version: Java 8mvnw --versionorgradlew --version): 3.6.3Additional context
(Add any other context about the problem here.)
The text was updated successfully, but these errors were encountered: