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

Upgrading from `@babel/parser@7.3.3` to `7.3.4` causes segfault/parser errors on Node 6 #9621

Closed
SimenB opened this issue Mar 1, 2019 · 21 comments

Comments

@SimenB
Copy link
Contributor

commented Mar 1, 2019

Bug Report

Current Behavior
Upgrading the Jest repo from @babel/parser@7.3.3 to @babel/parser@7.3.4 causes Node 6 to either seemingly stop transpiling (leading to syntax errors in every test) or just die with a segfault. I've spent most of today tweaking different versions in our lockfile to try to narrow down which version it regressed on, and I'm quite certain the issue is in 7.3.4 - I had 3 failures with it and 3 passes with 7.3.3 reverting between them with no other changes to the code.

You can see my adventure here: facebook/jest#8014

Most of the time it just seemingly stops parsing correctly, but sometimes it leads to segfault. I've never seen it fail on node 8, 10 or 11.

Input Code

You can see in that PR that the only change is using just @babel/parse@7.3.4, and it fails CI. Upgrading @babel/core as well makes no difference. CircleCI running Node 6 is the interesting CI - the other failures are either me cancelling builds or the CI provider cancelling redundant builds.

I'm sorry I'm not able to reduce it more - it happens locally for me as well on Node 6 (but not all the time) after a few minutes running the tests. Hopefully this is enough information for you to investigate. It's at least isolated to a patch release of a single package.

Expected behavior/code
I expect it to not fail :)

Babel Configuration (.babelrc, package.json, cli command)

https://github.com/facebook/jest/blob/ae8280f7b009c141b81576171ee784d336d09800/babel.config.js

// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.

module.exports = {
  babelrcRoots: ['examples/*'],
  overrides: [
    {
      presets: ['@babel/preset-flow'],
      test: '**/*.js',
    },
    {
      plugins: [
        'babel-plugin-typescript-strip-namespaces',
        require.resolve(
          './scripts/babel-plugin-jest-replace-ts-export-assignment.js'
        ),
      ],
      presets: ['@babel/preset-typescript'],
      test: /\.tsx?$/,
    },
  ],
  plugins: [
    ['@babel/plugin-transform-modules-commonjs', {allowTopLevelThis: true}],
    '@babel/plugin-transform-strict-mode',
  ],
  presets: [
    [
      '@babel/preset-env',
      {
        shippedProposals: true,
        targets: {node: 6},
      },
    ],
  ],
};

Environment

  • Babel version(s): core@7.3.3, parser@7.3.4
  • Node/npm version: Node 6, yarn 1.13.0
  • OS: macOS, but CI is Linux
  • Monorepo: Yes, Yarn Workspaces
  • How you are using Babel: babel-jest

Additional context/Screenshots
Stopped transpiling: https://circleci.com/gh/facebook/jest/54093
Segfault: https://circleci.com/gh/facebook/jest/54414

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Mar 1, 2019

Hey @SimenB! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@danez

This comment has been minimized.

Copy link
Member

commented Mar 2, 2019

I will have a look. There weren't that many changes between this two versions.

My guess is that the segfault is coming from jest which gets overwhelmed when thousands of tests fail. I experienced this (or at least similar behavior) also in babel when I make stupid changes that make every test explode.

Never the less will look into the Syntax errors.

@danez danez added pkg: parser and removed i: needs triage labels Mar 2, 2019

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

Thanks! Note that tests are happily passing for a few minutes before they start failing. So my guess would be some data is stored in memory, and it runs out/recurses badly at some point. That's totally a shot in the dark, though!

@danez

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

I can reproduce it locally with node 6, but yeah it only happens after a while. Running the failed tests alone works. 🤔

We are not storing some global state, we do not use WeakMaps and every parse call creates a new Parser instance.

Also the only difference in the parser package from 7.3.3 to 7.3.4 is this:

--- 	2019-03-03 00:17:50 +0000
+++ 	2019-03-03 00:17:50 +0000
@@ -8493,7 +8493,9 @@
   }
 
   checkDeclaration(node) {
-    if (node.type === "ObjectPattern") {
+    if (node.type === "Identifier") {
+      this.checkDuplicateExports(node, node.name);
+    } else if (node.type === "ObjectPattern") {
       for (let _i4 = 0, _node$properties = node.properties; _i4 < _node$properties.length; _i4++) {
         const prop = _node$properties[_i4];
         this.checkDeclaration(prop);
@@ -8510,8 +8512,8 @@
       this.checkDeclaration(node.value);
     } else if (node.type === "RestElement") {
       this.checkDeclaration(node.argument);
-    } else if (node.type === "Identifier") {
-      this.checkDuplicateExports(node, node.name);
+    } else if (node.type === "AssignmentPattern") {
+      this.checkDeclaration(node.left);
     }
   }
 
@@ -8730,7 +8732,7 @@
 
   tsNextTokenCanFollowModifier() {
     this.next();
-    return !this.hasPrecedingLineBreak() && !this.match(types.parenL) && !this.match(types.parenR) && !this.match(types.colon) && !this.match(types.eq) && !this.match(types.question);
+    return !this.hasPrecedingLineBreak() && !this.match(types.parenL) && !this.match(types.parenR) && !this.match(types.colon) && !this.match(types.eq) && !this.match(types.question) && !this.match(types.bang);
   }
 
   tsParseModifier(allowedModifiers) {
@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

I can reproduce it locally with node 6, but yeah it only happens after a while. Running the failed tests alone works. 🤔

Yeah, that's what I got as well. Makes it nearly impossible to start profiling etc.

Also the only difference in the parser package from 7.3.3 to 7.3.4 is this:

Unless checkDuplicateExports is doing something weird, I agree that seems super odd to impact this. However, it consistently fails on 7.3.4 and consistently passes on 7.3.3, so it doesn't seem that we're close to some limit that is crossed on 7.3.4. If so, I'd expect 7.3.3 to sometimes fail and 7.3.4 to sometimes pass, no?


I did DEBUG=babel but it output so much that I was unable to make heads or tails of it. But maybe you can understand it? It's so much output, could probably set debug's log function to something that writes to disk or something so it's easier to sift through

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

every parse call creates a new Parser instance.

You're certain it's properly GC-ed as well? IIRC we had to make sure to null out properties of certain objects/instances to fix memory leaks in Jest.

Jest includes a --detectLeaks flag, but IDK if it'd help in this case

@danez

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

It seems I did run in a similar issue in our tests here too: https://travis-ci.org/babel/babel/jobs/502253262

@danez danez added the Priority: High label Mar 5, 2019

@danez

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Jest includes a --detectLeaks flag, but IDK if it'd help in this case

I tested this and yes 50% of our tests emit a warning, but not sure how to figure out what is causing this.

@danez

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

I'm able to use the debugger and stop right at the point where the first error is thrown, but it all does not make any sense. In the debugger the internal state looks okay, but it seems that some comparisons didn't match although the should have. Also the generated error message then suddenly contains different values than what I see in the debugger. As if there are multiple states and i can only see one in the debugger.
I don't think there is anything we can do about, at least I don't see what.

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

If you see the debugging behaviour in Jest, is it due to workers? That you inspect in one thread, but there's another that actually explodes? ndb supports debugging subprocesses, maybe worth a try?

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 8, 2019

Does jest spawn multiple processes also with the run-in-band option?

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

In watch mode, yes: facebook/jest#6647

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

@SimenB I tried running rm -rf examples/react-native && yarn test-ci-partial after upgrading to @babel/parser@7.4.2 and the only failures I get are caused by mercurial not being installed on my pc and some

  ● tests different objects

    TypeError: weak is not a function

      at new weak (packages/jest-leak-detector/src/index.ts:41:5)
      at Array.map (native)

Can you still reproduce the bug?

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

Yes, see https://circleci.com/gh/facebook/jest/59227

Did you try running multiple times?

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

Only twice, I will check it again.

@Nuuou

This comment has been minimized.

Copy link

commented Mar 25, 2019

I'm experiencing this same behavior.

Some additional info:

  • OS: Ubuntu 18.04.2 LTS
  • Node: 6.14.1
  • NPM: 3.10.10
  • @babel/parser: 7.4.2 (tried with various versions >7.3.3, all same issue)

Similarly to above, it happens to me after a while, but it's fairly sporadic. Happening during any "watch" task I run. In particular, I have a fairly standard Webpack build process with Babel setup.

Also really happy I found this issue thread, as it's been really confusing trying to figure out why this all of a sudden started happening on a new project!

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

@Nuuou Do you have a repo we could look at? Maybe using an example smaller than Jest will make it easier to debug the problem.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Jest just dropped support for node 6 (thanks @JLHwung for noticing it!)

facebook/jest@e52bec9

@SimenB

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

Yeah, feel free to close this issue if you want. It's probably still an issue, but also probably not worth spending hours and hours on 🙂

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Yeah. Given that both me and @danez did already spend much time on this issue without concluding anything, that it doesn't happen when the debugger is opened (#9621 (comment)) and that the release which introduced this regression didn't have any related change (#9621 (comment)), I'd catecorize this issue as "too hard to fix".

Also, while we will still support node 6 until Babel 8, it is deprecated (https://github.com/nodejs/Release).

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Note that reducing memory consumption is still a goal, and we will gladly accept PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.