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 detected #5

Open
jkomyno opened this issue Dec 8, 2017 · 16 comments
Open

Memory leak detected #5

jkomyno opened this issue Dec 8, 2017 · 16 comments

Comments

@jkomyno
Copy link

jkomyno commented Dec 8, 2017

Preliminary note. I didn't installed inflight directly, it's just a package that glob uses.

$ npm list inflight
myLibrary@0.0.1 path/to/myLibrary
`-- glob@7.1.2
  `-- inflight@1.0.6

I'm developing a CLI apps that scans a set of files in the user's disk. Sometimes, it stops working throwing the following log:

<--- Last few GCs --->

[7828:000001CAE9906BF0]      729 ms: Mark-sweep 27.1 (94.3) -> [...] allocation failure GC in old space requested
[7828:000001CAE9906BF0]      754 ms: Mark-sweep 27.1 (94.3) -> [...]  last resort
[7828:000001CAE9906BF0]      780 ms: Mark-sweep 27.1 (32.3) -> [...] last resort


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 000002CEAE928799 <JSObject>
    1: _readdirEntries [path\to\myLibrary\node_modules\glob\glob.js:~559] [pc=000002F43C32198C](this=0000001978260B49 <Glob map = 000000777A7EB871>,abs=0000027C3514ED71 [...]

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory

So, I've inspected the Node's process' heap usage (node --inspect myLibrary.js) and got the following result, in which the inflight module is the most expensive package in terms of retained size.

memory leak

@Good-man
Copy link

Is this still an issue? I see that a fix was merged but the issue is still open.

@tysoncadenhead
Copy link

Can this issue be closed if it's been merged? I'd love to get the warning out of our security scanner

cmotsn pushed a commit to cmotsn/ngx-translate-extract that referenced this issue Apr 26, 2023
Upgrade the `glob` dependency to its latest version which does not rely anymore on the transitive dependency `inflight`. This is interesting because `inflight` contains a [memory-leak issue](isaacs/inflight#5) which is raised by Security scanners like Checkmarx.

ALso, glob@9 introduced Typescript support, so there is no need for the `@types/glob` package anymore.

As a consequence, the minimum Node version required is 16 because support for older versions was dropped by glob@9.
cmotsn pushed a commit to cmotsn/ngx-translate-extract that referenced this issue Apr 26, 2023
Upgrade the `glob` dependency to its latest version which does not rely anymore on the transitive dependency `inflight`. This is interesting because `inflight` contains a [memory-leak issue](isaacs/inflight#5) which is raised by Security scanners like Checkmarx.

ALso, glob@9 introduced Typescript support, so there is no need for the `@types/glob` package anymore.

As a consequence, the minimum Node version required is 16 because support for older versions was dropped by glob@9.
@JasoonX
Copy link

JasoonX commented Dec 5, 2023

In our project, we resolved this issue by introducing a pre-installation step that utilizes 'npm-force-resolutions'.
This approach was complemented by adding 'glob' to the 'resolutions' section within our 'package.json'.
This modification effectively enforces the use of a newer version of 'glob'.

package.json

{
  ...
  "scripts": {
    ...
    "preinstall": "npx npm-force-resolutions",
  },
  "resolutions": {
    "glob": "10.3.10"
  }
  ...
}

@thetumper
Copy link

Issue and PR in help-me, which resolves transitive issue in pino-pretty:

mcollina/help-me#17
mcollina/help-me#18

Please give a thumbs up there, to help get some priority on it.

@IdanAdar
Copy link

Snyk at it again. 🙄

@adominguezepiuse
Copy link

I've been following this issue, and I can see that it has been open for a considerable amount of time. I understand that addressing memory leak issues can be complex and time-consuming.

I was wondering if there has been any recent progress in resolving this problem. Is there any update or new information that can be shared? I'm interested in staying informed about the current status of the situation.

If there are specific steps to reproduce the problem or any additional information that could help, I'd appreciate the details.

@rreeves8
Copy link

I've been following this issue, and I can see that it has been open for a considerable amount of time. I understand that addressing memory leak issues can be complex and time-consuming.

I was wondering if there has been any recent progress in resolving this problem. Is there any update or new information that can be shared? I'm interested in staying informed about the current status of the situation.

If there are specific steps to reproduce the problem or any additional information that could help, I'd appreciate the details.

From my understanding the biggest problem is glob version 4 uses this package. Which in turn effects a large number of packages. However glob version 10 no longer uses inflight. So it's just a matter of encouraging package maintainers to update there dependencies to resolve it.

@ramdaniAli
Copy link

ramdaniAli commented Dec 22, 2023

using npm-force-resolutions doesn't fundamentally resolve the issue since running npm ls glob may still show that the old, vulnerable version of the glob package is present in your dependency tree. A more robust solution is to leverage the overrides feature in your package.json. By explicitly specifying an override for glob, like this:

{
  ...
  "overrides": {
    "glob": "10.3.10"
  },
  ...
}

You ensure that the specified version of glob is applied throughout your project. However, for this approach to take effect, you will need to perform a fresh installation, you should delete the node_modules folder and the package-lock.json file, and then run npm install. This process forces npm to reinstall all dependencies, taking into account the override you've specified for glob. After doing this, Snyk should no longer report any warnings related to glob, confirming that the chosen version is now in use without any remnants of the old version.

@rreeves8
Copy link

rreeves8 commented Dec 22, 2023

using npm-force-resolutions doesn't fundamentally resolve the issue since running npm ls glob may still show that the old, vulnerable version of the glob package is present in your dependency tree. A more robust solution is to leverage the overrides feature in your package.json. By explicitly specifying an override for glob, like this:


{

  ...

  "overrides": {

    "glob": "10.3.10"

  },

  ...

}



You ensure that the specified version of glob is applied throughout your project. However, for this approach to take effect, you will need to perform a fresh installation, you should delete the node_modules folder and the package-lock.json file, and then run npm install. This process forces npm to reinstall all dependencies, taking into account the override you've specified for glob. After doing this, Snyk should no longer report any warnings related to glob, confirming that the chosen version is now in use without any remnants of the old version.

This isn't a robust solution either. Overriding dependency versions can create instability. Each package version is not exactly the same and changing it for every use case will most likely break stuff.

@ramdaniAli
Copy link

Certainly, overriding a sub dependency versions carries inherent risks, such as the introduction of unpredictable behavior or a destabilization of system components. Despite these concerns, in my specific situation where glob functions solely as a dependent of argon2, the associated risks are somewhat contained and manageable. At present, our most prudent course of action would be to remain vigilant for updates issued by the argon2 maintainers, in the hope that they will provide a comprehensive solution to the dependency concern.

@davidguilherme
Copy link

Glob package has a breaking change from version 8.x (latest with inflight) to 9.x (first without it). They changed its API to use promises instead of callbacks. I believe the best approach is to patch 8.x version to stop using inflight which will make a lot easier to override it in the package.json.

@MandeeGit
Copy link

we are seeing the same issue in checkmarx as well. is this false positive?

CWE-772

image

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

No branches or pull requests