node-graceful-fs: 4.2.2 causes stack overflows in Jest

Something in the bump from 4.2.1 -> 4.2.2 is causing stack overflow issues with Jest. Jest uses graceful-fs.

(Found originally when reading this comment thread: https://github.com/isaacs/node-graceful-fs/commit/9841d473354e8894e142e5216f8eec93a581f3e1#r34841595 )

Using pinned versions of jest (24.8, 24.9) and simply using < 4.2.2 and 4.2.2 goes from test harnesses running to failing, both in setup steps as well as text execution

e.g.

    RangeError: Maximum call stack size exceeded
        at Function.[Symbol.hasInstance] (<anonymous>)

      at ReadStream (../../node_modules/graceful-fs/graceful-fs.js:177:14)
      at ReadStream (../../node_modules/graceful-fs/graceful-fs.js:178:28)
      at ReadStream (../../node_modules/graceful-fs/graceful-fs.js:178:28)
      at ReadStream (../../node_modules/graceful-fs/graceful-fs.js:178:28)
      at ReadStream (../../node_modules/graceful-fs/graceful-fs.js:178:28)
      ...

Obviously this depends on the tests being executed (simple tests don’t fail).

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 13
Comments: 16 (7 by maintainers)

Commits related to this issue

Split legacy stream properties from regular ones. Fixes #174 — committed to coreyfarrell/node-graceful-fs by coreyfarrell 5 years ago
Fix build issue do to graceful-fs bug See: [4.2.2 causes stack overflows in Jest · Issue #174 · isaacs/node-graceful-fs](https://github.com/isaacs/node-graceful-fs/issues/174) cspell:ignore isaacs — committed to streetsidesoftware/cspell by Jason3S 5 years ago

Most upvoted comments

This seems to be related to having two graceful-fs packages imported from different locations on disk. I caught it in the debugger here, and i see patch in a 4.2.2 one being run first, then with another dependency, patch in 4.1.15 is being called.

If I symlink both to the same 4.2.2, I no longer get the issue.

However, if both modules have their own 4.2.2 in their respective node_modules folders, the issue persists.

It appears 4.2.2 triggers the problem when graceful-fs is loaded from two locations, even if only one of them is 4.2.2. Possibly only when the 4.2.2 one is loaded in first, but maybe not. Sorry I don’t have a sample to run.

fcrick on Aug 30, 2019

Ok, so, here’s what I need for a fix to happen on this.

First, most importantly, we need a reproducible test case. A lot of folks are chiming in here saying it’s happening to them, in CI/CD, in their applications, etc. Give me a case that fails, a repo I can clone and a command to run to demonstrate the problem.

People have pointed out that it’s related to the global queue, or to jest blowing away the global object. I’m not going to just run on the hypothesis that it’s related to the global singleton queue, without some test so I can verify that it doesn’t happen when putting the queue on the fs module object as well.

It seems to only affect people using Jest. At the risk of seeming insensitive, I don’t maintain Jest. It doesn’t affect tap, the test framework I do maintain (and maintain quite actively, considering it’s not my paying job). So, “this module (which works fine everywhere else) is a problem for jest” really sounds more like a “jest” problem than a “me” problem.

If you provide me with a reproducible test case, I’ll fix this as soon as I get to it, assuming I can turn that reproducible test case into a failing test.

Graceful-fs is also used by npm, tap, nyc, and lots of other extremely popular and widely used libraries, where the commit in question fixes a pretty bad bug with leaked file descriptors when graceful-fs is loaded multiple times. Jest appears to be the only one broken by this, and I can’t honestly say whether that’s due to something inappropriate or dangerous that Jest is doing, or if there’s actually a problem here. If we’re going to roll back that fix, or explore some other way to do it, we need to do so properly. That means: with a test that fails before the patch, fixed in such a way that it doesn’t break any other tests.

If you don’t want to wait around for “as soon as I can get to it”, then the best alternative is to submit a patch, with a failing test demonstrating the issue.

If you don’t want to wait, and don’t have time to come up with a reproducible test case of your own, then you can pay me to prioritize this work ahead of the other things going on in my life. I still will need a reproducible case to poke at, but it doesn’t have to be nice, and I’ll get right on it if I can justify the personal expense.

If you don’t have time and attention to spend on this, and also don’t have money to spend on it, I’m sorry, but there’s little I can do responsibly. I recommend bugging the Jest project to see if there’s any help they can provide.

isaacs on Oct 22, 2019

@isaacs Are there plans to release a fix to this bug? Many packages rely on this one and jest randomly breaking when running fs module code has broken my team’s CI/CD reliability.

mrabuse on Oct 22, 2019