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

Memory leak in node-linkedlist #601

Closed
x3cion opened this issue Mar 20, 2019 · 4 comments · Fixed by #609
Closed

Memory leak in node-linkedlist #601

x3cion opened this issue Mar 20, 2019 · 4 comments · Fixed by #609
Assignees
Labels
Bug A code defect that needs to be fixed. semver-major This issue requires backwards-incompatible changes to address.

Comments

@x3cion
Copy link
Contributor

x3cion commented Mar 20, 2019

Hey there,

I created a pull request ( kilianc/node-linkedlist#7 ) for node-linkedlist quite some time ago, but it's not accepted yet and it seems the project is not very active.

The package has a memory leak that I found because I'm using arangojs. On shift(), the reference to the old head is not removed, thus keeping all old heads as long as there's pressure on the queue.

You might want to integrate the whole library (with my fix) or switch to another implementation.

Hope you can fix this soon, so I can drop my workaround github dependency.

@pluma pluma self-assigned this Mar 20, 2019
@pluma pluma added Bug A code defect that needs to be fixed. semver-major This issue requires backwards-incompatible changes to address. labels Mar 20, 2019
@pluma
Copy link
Contributor

pluma commented Mar 20, 2019

Thanks. The project hasn't been touched in years and uses some outdated APIs. I think I'll inline it and apply the changes.

@breakbild
Copy link

Looks like only Array like methods (push/shift/length) are used on _queue. As a quick fix I replaced this._queue = new LinkedList(); with this._queue = []; in class Connection (connection.js).

@pluma
Copy link
Contributor

pluma commented Apr 29, 2019

@breakbild the problem is that unintuitive as it may seem, LinkedList is significantly faster than native array operations. I tried replacing it but the slowdown was noticeable when performing a large number of queries at the same time.

@x3cion
Copy link
Contributor Author

x3cion commented Sep 28, 2019

I merged master and the pull request #609 is conflict free again. I also updated my library.

I'm still using my linkedlist branch in production environments to override the broken linkedlist implementation, which should not be the way to do things. Please let me know if I can help somehow.

pluma added a commit that referenced this issue Oct 18, 2019
altras added a commit to altras/orango that referenced this issue Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A code defect that needs to be fixed. semver-major This issue requires backwards-incompatible changes to address.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants