search

brianc / node-pg-pool

A connection pool for node-postgres
MIT License
180 stars 64 forks source link

Prevent double release with callback #115

Closed johanneswuerbach closed 5 years ago

johanneswuerbach commented 5 years ago

When using the done callback instead of client.release, double releasing a client was possible causing clients to be re-added multiple times.

Ensure that the release method itself can't be called twice instead of patching on the client.

I also did a small refactoring to ensure that new client and re-used client acquiring goes through the same code path and moved release into the client instead of using bind. Happy to remove this, but I found the logic a bit more readable afterwards.

brianc commented 5 years ago

Code changes look good. Thanks for diving in here. Question is....do you think this is a minor or major version bump? I feel more so like it's minor as the old behavior was undefined or would introduce silent bugs...but it does change the behavior. What you think?

johanneswuerbach commented 5 years ago

I would see this as a (critical) bug fix as releasing the same client multiple times will result in multiple idleQueue entries all referencing the same client, which will cause various other problems.

brianc commented 5 years ago

patch version then...yeah?

johanneswuerbach commented 5 years ago

I would say so, the same behaviour was already happening when calling client.release() twice.

olgkpln commented 5 years ago

@brianc any idea when you plan to merge this? I would like to use it on my project.

johanneswuerbach commented 5 years ago

@brianc friendly ping :-)

johanneswuerbach commented 5 years ago

@brianc anything else required to move this forward?

johanneswuerbach commented 5 years ago

Fixed in https://github.com/brianc/node-pg-pool/pull/123