Fix module Load order and re-fix circular loading logic

[rhuanjl]

Fixes #5171 but this may take a bit of explaining.

@boingoing please could you take a look at this?

Note I've targeted this at master - though would prefer to target 1.8 if allowed as this is fixing a regression that occurred in 1.8 I've targeted master as I'm assuming 1.8 is closed to functionality changes at this point.

This PR reverts 2dc946c which apparently fixed some circular module loading bugs by reversing module load order.

However the reversed load order resulted in observably different runtime results in some cases - introducing an incompatibility with other runtimes described in issue #5171

As far as I understand the purpose of the order reversal was to resolve problems where circular imports were not handled correctly, these were:

1. Issue ChakraCore segfaults during JsParseModuleSource in certain cases involving circular imports #4482
   But since then ChakraCore segfaults during JsParseModuleSource in certain cases involving circular imports #4482 was fixed properly in Modules: Fix indirect circular children #4583 - after noting that the order reversal fixed the original POC but did not fix a slightly revised version of the same.
2. A non-specific issue that had resulted in the test-case bug-issue-3257 being disabled - this is an issue that can only occur with either an environment that allows dynamic imports OR an environment that allows multiple root modules to be being loaded at the same time in the same context.

This PR includes an alternative fix for the issue that caused bug-issue-3257 to be disabled. And adds a test case that reproduces the same bug regardless of the load order.

The revised fix for that was to get rid of the concept of numPendingModules that was used to track when all of a module's children were ready.

Previous logic:

1. Module is parsed that has children
2. For each child:

• if child already parsed do nothing
• if child not already parsed increment numPendingModules for the parent and add parent to list of modules to notify when child is parsed

3. When a module is parsed that has no pending children notify any parents in its list
4. When a module is notified by a child decrease numPendingModules for it by 1, when it reaches 0 notify its parent OR if it's a root module or a dynamicly imported module instantiate it and notify the host to execute it

Problem with previous logic

1. Mod0 parsed, has children Mod1 and Mod2 (schedule async parse job for Mod1 and Mod2)
2. Mod1 also a root module or dynamic module and has Mod0 as a child
3. parse job for Mod1 completes -> notes that Mod0 is already parsed so numPendingModules = 0
4. Host notified to execute Mod1 before Mod2 has been parsed

New logic

1. Module is parsed that has children
2. For each child:

• add parent to list of modules to notify when child is parsed

3. Whenever a module is parsed OR a notification is received:

• recursively check if all children and children's children etc. have been parsed
• if yes notify parent OR if dynamic/root module -> notify host to execute

[sethbrenith]

LGTM but I'll wait for someone who knows the area better to sign off. Thanks for the fix!

[boingoing]

Looks good to me! Thanks for fixing this one.

[rhuanjl]

@boingoing thanks for the review, I've updated to latest and added an extra test case as thought it could do with a little extra coverage, please let me know if you'd like me to do anything else.

[boingoing]

@rhuanjl Thanks for the work, Richard, I will attempt to land this for you this week. Would you mind rebasing once again?

[boingoing]

@rhuanjl Thank you very much for contributing, I have landed this in master. Unfortunately it won't be taken into 1.8 - that ship has sailed at this point.

[rhuanjl]

Thanks for landing this. Understood about it not being able to go to 1.8 - didn't really think it could but thought I should at least ask.