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.

Sign up for GitHub

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

Jump to bottom

Fix data reader not getting disposed #4258

Merged
merged 1 commit into from
Nov 11, 2020
Merged

Fix data reader not getting disposed #4258

merged 1 commit into from
Nov 11, 2020

Conversation

bdukes
Copy link
Contributor

@bdukes bdukes commented Oct 28, 2020

Fixes #3840

@bdukes bdukes added this to the 9.8.1 milestone Oct 28, 2020
valadas
valadas approved these changes Oct 28, 2020
View reviewed changes
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Looks good to me, basically just to learn something from this, it's the change to remove static that makes it disposable and the rest is to support that change right?

@bdukes
Copy link
Contributor Author

bdukes commented Oct 28, 2020

No, the static change was just because the method was reimplementing an instance method for no good reason. The fix is the switch from ExecuteReader (which returns an IDataReader that needs to be disposed) to ExecuteNonQuery which does handles disposing the connection/reader on its own.

@valadas
Copy link
Contributor

valadas commented Oct 28, 2020

Ha thanks for the info, makes sense.

@sleupold sleupold mentioned this pull request Nov 11, 2020
Closed
2 tasks
@valadas valadas merged commit 9ff814d into dnnsoftware:develop Nov 11, 2020
donker added a commit to donker/Dnn.Platform that referenced this pull request Nov 13, 2020
@donker
Roll back PR dnnsoftware#4258 as it broke the data provider
7fbfc7a
@donker donker mentioned this pull request Nov 13, 2020
Closed
@bdukes bdukes mentioned this pull request Nov 13, 2020
Merged
@bdukes bdukes deleted the fix-unclosed-datareader branch November 30, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mitchelsellers mitchelsellers mitchelsellers approved these changes

@valadas valadas valadas approved these changes

Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
9.8.1
Development

Successfully merging this pull request may close these issues.

SqlDataProvider.IsConnectionValid leaks connections, exhausts pool
3 participants
@bdukes @valadas @mitchelsellers