Moleculer: Circular dependency loop possible hot-reload middleware

Created on 22 Feb 2021  路  3Comments  路  Source: moleculerjs/moleculer

  • [x] I am running the latest version (0.14.12)
  • [x] I checked the documentation and found no answer
  • [x] I checked to make sure that this issue has not already been filed
  • [x] I'm reporting the issue to the correct repository

Current Behavior

Circular dependency loop is possible in processModule function in the hot-reload middleware, causing a loop in the runner command. I did some poking around and logged the file names it was importing and it got stuck in some sort of circular dependency loop:

/workspace/dist/entity/job/JobImport.js
/workspace/dist/lib/orm.js
/workspace/dist/util/entity.js
/workspace/dist/lib/errors.js
/workspace/dist/lib/errors/http.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/code.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/errors/api.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/errors/http.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/code.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/util/date.js
/workspace/dist/lib/orm.js
/workspace/dist/util/entity.js
/workspace/dist/lib/errors.js
/workspace/dist/lib/errors/http.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/code.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/errors/api.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/errors/http.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/code.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/lib/http/status.js
/workspace/dist/lib/http/status/message.js
/workspace/dist/util/date.js
... repeats

Steps to Reproduce

I can reproduce this on my code base, but I can't figure out how it has become circular. I can not share my code base as its is closed source. I am using typeorm and I have a ton of potential circular dependencies.

  • Moleculer version: 0.14.12
  • NodeJS version: 14x
  • Operating System: Linux fa60ef954fce 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 GNU/Linux

Most helpful comment

Hi @nurdism

After looking at the repo that you've provided and debugging the hot-reload middleware I think that everything is working fine. Still, I'll try to explain the issue that you're facing.

Due to large number of require present in your code base and due to recursive nature of processModule function the hot-reload.js simply can't build, within a reasonable amount of time, a proper file dependency tree.

I've searched for require( in the dist/entity folder and here's the result:
unknown (1)

There are 50 files and 200 require declarations. Moreover, most of them are circular. Just an example:
image

However, the issue is not the circular deps because of these lines over here:
https://github.com/moleculerjs/moleculer/blob/fb0aa7e2a2c7f586f4418ee6778d9c410e54825a/src/middlewares/hot-reload.js#L229-L231

Even if we abstract ourselves from circular imports and just look at deps tree of User.ts we'll see that just to create a dependency tree for User we have to do 7 processModule calls. Then, only to expand the Application dep of User we have to do additional 12 processModule calls. Then, going one level deeper and expanding the Message dep of Application we have to do additional 8 processModule calls. You get the idea...

image

As you can see, the way that you've structured things generates an exponential number of recursive calls
https://github.com/moleculerjs/moleculer/blob/fb0aa7e2a2c7f586f4418ee6778d9c410e54825a/src/middlewares/hot-reload.js#L275

To test my assumption I've used a tool called dependency-cruiser

Tested it for the moleculer repo, which has a pretty large codebase but the imports are structured in a simple way, and it took around 1 sec. to generate the deps tree.

Next, I've tried to do the same thing for the code base that you've provided. Waited for 1 hour and got nothing.... I don't know the exact implementation of dependency-cruiser but it has to do the same thing as the processModule, i.e., it has to recursively build a deps tree for each file.

My next attempt was to try to reduce the deps (by removing some files from dist/entity) and once I did it both (dependency-cruiser and hot-reload middleware) started to work properly.

Having said that, your PR https://github.com/moleculerjs/moleculer/pull/876 did not solve things. What it did was breaking the recursion before the entire deps tree was built.

All 3 comments

Thanks for the report, but before a PR, we need a reproduce example to detect the root of the issue. No need the source of the project, just a minimal repro code.

Hi @nurdism

After looking at the repo that you've provided and debugging the hot-reload middleware I think that everything is working fine. Still, I'll try to explain the issue that you're facing.

Due to large number of require present in your code base and due to recursive nature of processModule function the hot-reload.js simply can't build, within a reasonable amount of time, a proper file dependency tree.

I've searched for require( in the dist/entity folder and here's the result:
unknown (1)

There are 50 files and 200 require declarations. Moreover, most of them are circular. Just an example:
image

However, the issue is not the circular deps because of these lines over here:
https://github.com/moleculerjs/moleculer/blob/fb0aa7e2a2c7f586f4418ee6778d9c410e54825a/src/middlewares/hot-reload.js#L229-L231

Even if we abstract ourselves from circular imports and just look at deps tree of User.ts we'll see that just to create a dependency tree for User we have to do 7 processModule calls. Then, only to expand the Application dep of User we have to do additional 12 processModule calls. Then, going one level deeper and expanding the Message dep of Application we have to do additional 8 processModule calls. You get the idea...

image

As you can see, the way that you've structured things generates an exponential number of recursive calls
https://github.com/moleculerjs/moleculer/blob/fb0aa7e2a2c7f586f4418ee6778d9c410e54825a/src/middlewares/hot-reload.js#L275

To test my assumption I've used a tool called dependency-cruiser

Tested it for the moleculer repo, which has a pretty large codebase but the imports are structured in a simple way, and it took around 1 sec. to generate the deps tree.

Next, I've tried to do the same thing for the code base that you've provided. Waited for 1 hour and got nothing.... I don't know the exact implementation of dependency-cruiser but it has to do the same thing as the processModule, i.e., it has to recursively build a deps tree for each file.

My next attempt was to try to reduce the deps (by removing some files from dist/entity) and once I did it both (dependency-cruiser and hot-reload middleware) started to work properly.

Having said that, your PR https://github.com/moleculerjs/moleculer/pull/876 did not solve things. What it did was breaking the recursion before the entire deps tree was built.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

demetriusnunes picture demetriusnunes  路  5Comments

rozhddmi picture rozhddmi  路  4Comments

slinkardbrandon picture slinkardbrandon  路  4Comments

DeividasJackus picture DeividasJackus  路  4Comments

ngraef picture ngraef  路  4Comments