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

Connection not closed when database name is incorrect #173 fix #224

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

parMaster
Copy link

#173 describes a bug when a database name is provided that does not exist, but the connections remain open and db.Close() doesn't help to close them.
This PR is supposed to fix this issue by closing connection before returning error. The connection is unusable despite the message from the server like Cannot open database "shmaster" that was requested by the login. Using the user default database "master" instead - db.PingContext returns an error, but connections remain open

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.83%. Comparing base (02deabf) to head (baba640).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
+ Coverage   74.62%   74.83%   +0.20%     
==========================================
  Files          32       32              
  Lines        6396     6397       +1     
==========================================
+ Hits         4773     4787      +14     
+ Misses       1335     1327       -8     
+ Partials      288      283       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shueybubbles
Copy link
Collaborator

thx for the PR

Please add a test that fails without your change.

@parMaster
Copy link
Author

thx for the PR

Please add a test that fails without your change.

You're right.
As you can see in the original issue, we can monitor the opened connections with a netstat -an | grep '1.1433' cmd, but I think it's not right to call exec.Command("netstat", "-an") and check the output.
So, I've decided to query a sys.dm_exec_connections system view which Returns information about the connections established to this instance of the database engine and check if there are leaked connections left even after explicit conn.Close().
The test fails without my change and shows the number of leaked connections.

tds_test.go Outdated Show resolved Hide resolved
@parMaster
Copy link
Author

@microsoft-github-policy-service agree company="Amoniac OÜ"

tds_test.go Outdated Show resolved Hide resolved
@shueybubbles shueybubbles merged commit 573423d into microsoft:main Oct 14, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants