Skip to content

Fix memory leak in process.namespaces#35

Open
tlhunter wants to merge 1 commit intoJeff-Lewis:masterfrom
tlhunter:memory-leak
Open

Fix memory leak in process.namespaces#35
tlhunter wants to merge 1 commit intoJeff-Lewis:masterfrom
tlhunter:memory-leak

Conversation

@tlhunter
Copy link
Copy Markdown

Numeric namespace identifiers are never removed from the global process.namespaces object:

process.namespaces[name] = null;

process.namespaces[name] = null;

@tlhunter
Copy link
Copy Markdown
Author

^ I believe the test runner is having issues. The same failures occur locally in master, as well as other recent open PRs.

@MichaelRBond
Copy link
Copy Markdown

This branch significantly reduces the memory creep I've been seeing with cls-hooked.

@kibertoad
Copy link
Copy Markdown

@Jeff-Lewis Any chance this could be looked at? Btw, if you don't have time for the project anymore (which may be the case judging by amount of commits lately), I would be more than happy to volunteer to help maintain the project.

@kibertoad
Copy link
Copy Markdown

Any updates?..

@RemyLespagnol
Copy link
Copy Markdown

RemyLespagnol commented Dec 30, 2019

@Jeff-Lewis Any chance to take a look at this?

In production :

image

@alexgarbarev
Copy link
Copy Markdown

It looks like problem with leak is not in "process.namespaces" but in async_hook leaking. See #60 for details.

@orloffv
Copy link
Copy Markdown

orloffv commented Nov 12, 2020

@Jeff-Lewis ping(

@ridakk
Copy link
Copy Markdown

ridakk commented May 21, 2021

ayn update on this ? 👀

@kibertoad
Copy link
Copy Markdown

@ridakk It seems to be pretty abandoned. Either way, if you are on modern enough Node, migrating to AsyncLocalStorage is preferable.
In case you want to ease up migration from cls-hooked, you could use a compatibility bridge: https://github.com/kibertoad/asynchronous-local-storage

@ridakk
Copy link
Copy Markdown

ridakk commented May 21, 2021

@ridakk It seems to be pretty abandoned. Either way, if you are on modern enough Node, migrating to AsyncLocalStorage is preferable.
In case you want to ease up migration from cls-hooked, you could use a compatibility bridge: https://github.com/kibertoad/asynchronous-local-storage

@kibertoad thx for the heads up, and yes we are on latest lts but our problem is Sequelize and it still depends on cls-hooked 😩

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.

7 participants