codemodsquad / asyncify

Don't keep your promises 😉
MIT License
13 stars 6 forks source link

Process hangs with 100% CPU utilization #24

Closed raphinesse closed 3 years ago

raphinesse commented 3 years ago

First of all thanks for the great work! This code mod seems to do a fantastic job from what I saw up until now.

Unfortunately there was a problem with one particular source file. To reproduce:

git clone https://github.com/apache/cordova-android.git
cd cordova-android
npm i jscodeshift @codemodsquad/asyncify
npx jscodeshift --verbose=2 -t node_modules/@codemodsquad/asyncify/index.js bin/templates/cordova/lib/check_reqs.js

There is no output from the process and it stays at 100% CPU utilization indefinitely (well, after 20 min or so I killed it).

jedwards1211 commented 3 years ago

oh no, infinite loops 😅 okay, I'll look into it!

jedwards1211 commented 3 years ago

@raphinesse btw you could avoid using npx in cases where you've installed the packages locally if you add ./node_modules/.bin to your PATH

jedwards1211 commented 3 years ago

:tada: This issue has been resolved in version 2.0.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jedwards1211 commented 3 years ago

Okay, the following code in check_reqs.js was obviously throwing it off. asyncify was walking the scope bindings for checkFn to see if it got to a function declaration, but it was getting stuck in an infinite loop when it reached checkFn, because I am dumb 😃 So I made sure to exit that loop when the next step would be the same node, and it works now.

Just, FYI asyncify isn't smart enough to know that checkFn is definitely a function here even though developers can obviously tell. If asyncify had found a function declaration named checkFn it would have unrolled promise.then(checkFn) into await checkFn(await promise).

    // Then execute requirement checks one-by-one
    return checkFns.reduce(function (promise, checkFn, idx) {
        // Update each requirement with results
        var requirement = requirements[idx];
        return promise.then(checkFn).then(function (version) {
            requirement.installed = true;
            requirement.metadata.version = version;
        }, function (err) {
            requirement.metadata.reason = err instanceof Error ? err.message : err;
        });
    }, Promise.resolve()).then(function () {
        // When chain is completed, return requirements array to upstream API
        return requirements;
    });
raphinesse commented 3 years ago

First of all: Thanks for the quick response and fix!

Now I am facing a really strange problem: I am still seeing the same infinite loop when transforming check_reqs.js with 2.0.5. If I debug the hanging process using

NODE_OPTIONS='--inspect' ./node_modules/.bin/jscodeshift --run-in-band -t node_modules/@codemodsquad/asyncify/index.js bin/templates/cordova/lib/check_reqs.js

then the debugger shows me the following code for canDefinitelyInvoke, which is missing the fix:

function canDefinitelyInvoke(expr) {
  if (expr.isIdentifier()) {
    var target = expr;

    while (target) {
      if (target.isIdentifier()) {
        var _target$scope$getBind;

        target = (_target$scope$getBind = target.scope.getBinding(target.node.name)) === null || _target$scope$getBind === void 0 ? void 0 : _target$scope$getBind.path;
      } else if (target.isVariableDeclarator()) {
        target = target.get('init');
      } else {
        break;
      }
    }

    return target ? target.isFunction() : false;
  }

  return expr.isFunction();
}

However, when I grep all the occurrences of that function definition, everything looks as it should:

$ rg -A 10 "function canDefinitelyInvoke" node_modules/
node_modules/@codemodsquad/asyncify/util/canDefinitelyInvoke.js
8:function canDefinitelyInvoke(expr) {
9-  if (expr.isIdentifier()) {
10-    var target = expr;
11-
12-    while (target) {
13-      if (target.isIdentifier()) {
14-        var _target$scope$getBind;
15-
16-        var nextTarget = (_target$scope$getBind = target.scope.getBinding(target.node.name)) === null || _target$scope$getBind === void 0 ? void 0 : _target$scope$getBind.path;
17-        if (nextTarget === target) break;
18-        target = nextTarget;

node_modules/@codemodsquad/asyncify/util/canDefinitelyInvoke.d.ts
3:export default function canDefinitelyInvoke<T extends t.Node>(expr: NodePath<T>): boolean;

node_modules/@codemodsquad/asyncify/es/util/canDefinitelyInvoke.js
8:function canDefinitelyInvoke(expr) {
9-  if (expr.isIdentifier()) {
10-    let target = expr;
11-
12-    while (target) {
13-      if (target.isIdentifier()) {
14-        var _target$scope$getBind;
15-
16-        const nextTarget = (_target$scope$getBind = target.scope.getBinding(target.node.name)) === null || _target$scope$getBind === void 0 ? void 0 : _target$scope$getBind.path;
17-        if (nextTarget === target) break;
18-        target = nextTarget;

I don't get what's happening here :confused:

raphinesse commented 3 years ago

Update: Running jscodeshift with --no-babel (i.e. do not apply babeljs to the transform file) fixed this for me. Probably a stale code cache somewhere :unamused:

Thanks again for the quick fix and sorry for the bother with this cache issue.