Nested imports while importing higher than 2 level modules will panic

[CYBAI]

A different panic when running the live test-suite at http://gpuweb-cts-glsl.github.io/standalone/. The code rests in branch glsl-dependent at https://github.com/gpuweb/cts/tree/glsl-dependent. This one's intermittent though.

DomRefCell<T> already borrowed: BorrowMutError (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at components/script/dom/bindings/cell.rs:93)
   0: servo::backtrace::print
   1: servo::main::{{closure}}
   2: std::panicking::rust_panic_with_hook
             at /rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libstd/panicking.rs:481
   3: rust_begin_unwind
             at /rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libstd/panicking.rs:385
   4: core::panicking::panic_fmt
             at /rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libcore/panicking.rs:89
   5: core::option::expect_none_failed
             at /rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libcore/option.rs:1272
   6: script::dom::globalscope::GlobalScope::set_module_map
   7: script::script_module::fetch_single_module_script
   8: script::script_module::host_import_module_dynamically
   9: _ZN2js24StartDynamicModuleImportEP9JSContextN2JS6HandleIP8JSScriptEENS3_INS2_5ValueEEE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/builtin/ModuleObject.cpp:1752
  10: _ZL9InterpretP9JSContextRN2js8RunStateE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/vm/Interpreter.cpp:4286
  11: _ZN2js9RunScriptEP9JSContextRNS_8RunStateE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/vm/Interpreter.cpp:423
  12: _ZN2js13ExecuteKernelEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectRKNS2_5ValueENS_16AbstractFramePtrEPS9_
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/vm/Interpreter.cpp:810
  13: _ZN2js7ExecuteEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectPNS2_5ValueE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/vm/Interpreter.cpp:843
  14: _ZN2js12ModuleObject7executeEP9JSContextN2JS6HandleIPS0_EENS3_13MutableHandleINS3_5ValueEEE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/builtin/ModuleObject.cpp:1076
  15: _ZL23intrinsic_ExecuteModuleP9JSContextjPN2JS5ValueE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/vm/SelfHosting.cpp:1976
  16: <unknown>
  17: <unknown>
  18: <unknown>
  19: _ZL13EnterBaselineP9JSContextRN2js3jit12EnterJitDataE
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/jit/BaselineJIT.cpp:113
      _ZN2js3jit32EnterBaselineInterpreterAtBranchEP9JSContextPNS_16InterpreterFrameEPh
             at /home/kunal/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/9a6d8fc/mozjs/js/src/jit/BaselineJIT.cpp:186
  20: <unknown>
  21: <unknown>
[2020-06-14T14:24:09Z ERROR servo] DomRefCell<T> already borrowed: BorrowMutError
fatal runtime error: failed to initiate panic, error 5
Redirecting call to abort() to mozalloc_abort

Stack trace for thread "ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }"
Servo exited with return value 11

@jdm iirc, I reported a similar error while running my test example initially. You fixed it on that occasion. I wonder if this one's related.

Originally posted by @kunalmohan in #25439 (comment)

[CYBAI]

With doing some investigation, I can confirm this is a regression after #26395 merged.

So, the issue is

a -> b -> c -> d -> e

e -> f -> g -> c -> d -> e

In #26395, we moved away from the recursive checking and checking if the dependencies are fetching or not by checking only 3 levels (in the above example, it will check a, b and c) and the circular dependency checking. However, in that way, when a descendant module is still under fetching but it's in a very deep level. Its status won't be checked.

I believe we can fix this issue if we can fix #26903 which means we make our implementation more spec-compliant and remove the checking.

However, I think WebGPU project needs the testing page to work and the #26903 needs more investigation.

So, I think the quickest way for now is let's rollback to use the recursive checking way.

I've tried locally and confirm the recursive way works fine and I also created a wpt test for it so that we won't miss this case again while trying to fix #26903.

@jdm @Manishearth does that make sense to you let's fix it to use with recursive checking module status and then remove the checking in #26903?

[jdm]

Reverting to a more-correct version sounds sensible to me.

[Manishearth]

Yeah, this was kinda why I was concerned about the GC approach

[CYBAI]

Yeah, this was kinda why I was concerned about the GC approach

Yeah! I can see the problem now >_< Hope I can fix #26903 soon...