Skip to content

Make sure dbcommand is disposed #2656

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

Merged
merged 2 commits into from
Feb 14, 2021

Conversation

RogerKratz
Copy link
Contributor

DbCommands created in GenericBatchingBatcher aren't disposed.

Copy link
Member

@fredericDelaporte fredericDelaporte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have qualified it as just a minor improvement instead of a major bug because in most cases with db commands, this disposal lack will not cause an unmanaged leak and will not prevent the command from being garbage collected.

If I am wrong, this fix should then be re-tagged, but also should then target 5.3.x and likely be back-ported down to the introduction of the generic batching batcher.

@RogerKratz
Copy link
Contributor Author

Agreed, I think it's minor as well, no unmanaged leak when SqlCommand is used I think. Not even sure it matters GC perf wise (maybe GC.SuppressFinalize is always called internally in SqlCommand anyhow, haven't looked closely)?

Still good to have it fixed (as "minor", yes).

@RogerKratz
Copy link
Contributor Author

....on the other hand it might cause an unmanged leak if an other DbCommand than SqlCommand is used. However, I don't know much about these, only thinking out loud now.

@hazzik
Copy link
Member

hazzik commented Feb 3, 2021

@fredericDelaporte maybe for 5.3.6?

@fredericDelaporte
Copy link
Member

Why not.

@fredericDelaporte fredericDelaporte changed the base branch from master to 5.3.x February 14, 2021 16:35
@fredericDelaporte
Copy link
Member

fredericDelaporte commented Feb 14, 2021

Changing the base did not went well, so I have done it manually with a forced push.

@fredericDelaporte fredericDelaporte modified the milestones: next minor, 5.3.6 Feb 14, 2021
@fredericDelaporte fredericDelaporte merged commit cbcca54 into nhibernate:5.3.x Feb 14, 2021
bahusoid pushed a commit to bahusoid/nhibernate-core that referenced this pull request Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants