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

Pool issue: give out more connections then maximal allowed! #7

Open
Apollon77 opened this issue May 24, 2022 · 0 comments · May be fixed by #8
Open

Pool issue: give out more connections then maximal allowed! #7

Apollon77 opened this issue May 24, 2022 · 0 comments · May be fixed by #8

Comments

@Apollon77
Copy link

Hi,

I needed some time to spot the issue in your Pool implementation. The current implementation is not considering the async nature of "providing an new connection" and so can easiely give out more connections as allowed.

Example: Assume I have a maximum pool of 20 connections I want to allow. And now I start fresh with 0 connections directly after "Open" call: When I now do a loop to borrow 200 connections then I will get them ... so was over the top

Reason is:
1 first you check if current active connections are already more then allowed done in https://github.com/intellinote/sql-client/blob/master/lib/sql-client-pool.coffee#L108-L110 is
2 if this is ok (and no free connection is in pool which can not be in my example) then you create a connection (which is asnyc!)
3 if it was ok then you increase active counter in https://github.com/intellinote/sql-client/blob/master/lib/sql-client-pool.coffee#L144

With my "loop of 200" the first check is always ok and so I get 200 connections ... and active is 200 afterwards

I would propose to always increase the counter before the check in assumption that you get a connection and decrease afterwrdas if this was a wrong assumption

Apollon77 added a commit to Apollon77/sql-client that referenced this issue May 24, 2022
Increase active counter in assumption to get the connection and to make sure to not give out too many connections
fixes intellinote#7
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 a pull request may close this issue.

1 participant