Skip to content

Fix memory leak #77

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

Closed
wants to merge 1 commit into from
Closed

Fix memory leak #77

wants to merge 1 commit into from

Conversation

scalm
Copy link

@scalm scalm commented May 12, 2020

I found a memory leak in the latest release.
Tested:
NodeJS v10.18.1, libpq 1.8.8 - No memory leak
NodeJS v10.18.1, libpq 1.8.9 - There is a memory leak
NodeJS v12.16.3, libpq 1.8.9 - There is a memory leak

I consulted the documentation and reference tests (https://github.com/nodejs/nan/blob/2c4ee8a32a299eada3cd6e468bbd0a473bfea96d/test/cpp/weak.cpp#L23
) and tried to fix.

brianc added a commit that referenced this pull request Aug 19, 2022
This fixes the issue outlined in #84 by preventing non-array values being passed into C/C++ binding from the JS layer.  Will modify node-pg-native & node-postgres individually to bump versions for this fix.

Also fixes memory leak from #77
brianc added a commit that referenced this pull request Aug 19, 2022
* Add check for valid parameter array

This fixes the issue outlined in #84 by preventing non-array values being passed into C/C++ binding from the JS layer.  Will modify node-pg-native & node-postgres individually to bump versions for this fix.

Also fixes memory leak from #77

* Add CI workflow to github

* Add postgres to ci

* Add logging to error so I can see whats going on in the githubs

* Use pretest so env vars go to test cmd

* Add host to cmd

* Run prettier with quote rule matching existing code for fewer changes

* Prettier that too
@brianc
Copy link
Owner

brianc commented May 30, 2024

This has been fixed - thanks!

@brianc brianc closed this May 30, 2024
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.

2 participants