Skip to content
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

Check a system property before EntityManager injection outside work unit happens #997

Conversation

nuzayats
Copy link

There are many folks who complains about implicit EntityManager injection because it very likely to bring EntityManager leakage:

#739
#730

I think it's reasonable to check a system property before implicit injection happens. it has no compatibility breakage, and simple.

Inspired by workaround of this: https://issues.apache.org/jira/browse/COLLECTIONS-580

…on before implicit EntityManager injection happens.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@nuzayats
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@nuzayats nuzayats changed the title Check a system property before implicit EntityManager injection happens. Check a system property before EntityManager injection outside work unit happens Feb 28, 2016
@nuzayats
Copy link
Author

@sameb
Hi, I guess you might thought this PR wouldn't be a correct solution to fix this issue. If so it's fine but anyway I believe the issue which this PR addressed should be fixed.

Why don't you make injection process of EntityManager to throw Exception when it happens outside unit of work as the message in Exception states? Do you afraid of compatibility breakage?

Any feedback or thoughts greatly appreciated :)

copybara-service bot pushed a commit that referenced this pull request Apr 21, 2023
…because it leads to leaks. Fixes #739, fixes #997, fixes #730, fixes #985, fixes #959, fixes #731. This also adds an optional Options to the JpaPersistModule constructor, if users wish to use the legacy behavior. This is a breaking change, but to a better default value. Users who want to keep the dangerous form can opt back in with the options.

PiperOrigin-RevId: 525852009
copybara-service bot pushed a commit that referenced this pull request Apr 21, 2023
…because it leads to leaks. Fixes #739, fixes #997, fixes #730, fixes #985, fixes #959, fixes #731. This also adds an optional Options to the JpaPersistModule constructor, if users wish to use the legacy behavior. This is a breaking change, but to a better default value. Users who want to keep the dangerous form can opt back in with the options.

PiperOrigin-RevId: 525852009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants