[patch][ccla] Infinite recursion loading missing module specified in __package__.js

[[email protected]…]

If you have a file __package__.js that looks like this:

dojo.kwCompoundRequire({
  common: [ 
    "my.example.AAA",
    "my.example.BBB",
    "my.example.p1.*"
  ]
});
dojo.provide("my.example.*");

and another:

dojo.kwCompoundRequire({
  common: [ 
    "my.example.p1.CCC",
    "my.example.p1.DDD",
    "my.example.p1.EEE"
  ]
});
dojo.provide("my.example.p1.*");

and one of those files is missing (eg. my/example/p1/DDD.js), then we get an infinite recursion bug during loading when the user does dojo.require("my.example.*")

loading order:

my/example/__package__.js
my/example/AAA.js
my/example/BBB.js
my/example/p1/__package__.js
my/example/p1/CCC.js
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
... forever ...

This is because the search order looks like this:

my/example/__package__.js
my/example/AAA.js
my/example/BBB.js
my/example/p1/__package__.js
my/example/p1/CCC.js
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
my/example/p1/__package__.js    --> already loaded, so start processing it again
my/example/p1/CCC.js    --> already loaded
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
my/example/p1/__package__.js    --> already loaded, so start processing it again
my/example/p1/CCC.js    --> already loaded
my/example/p1/DDD.js    --> 404
my/example/p1.js    --> 404
my/example/p1/__package__.js --> already loaded, so start processing it again
... forever ...

In loader.js line 291, there is an infinite recursion check which doesn't seem to do anything (nothing ever looks at addedToLoadingCount). Applying the following patch kills the infinite loop and brings up normal "can't load, tried xxx" errors.

Index: src/loader.js
===================================================================
--- src/loader.js       (revision 500)
+++ src/loader.js       (working copy)
@@ -289,8 +289,8 @@
        }

        // protect against infinite recursion from mutual dependencies
-       if(dj_undef(moduleName, this.loading_modules_)){
-               this.addedToLoadingCount.push(moduleName);
+       if(!dj_undef(moduleName, this.loading_modules_)){
+               return false;
        }
        this.loading_modules_[moduleName] = 1;

I've tested this against all the common use cases I can think of. However, this might require some further thought from people who know the loader better. And the unused addedtoLoadingCount and removedFromLoadingCount should probably be removed too. I also wonder whether when a module is successfully loaded it should be removed from loading_modules_ (since it seems to have everything ever loaded or loading in there).

[Adam Peller]

I think this all goes away with the new loading scheme in 0.9