Handle more partial-load states + fix possible dangling pointer.

==Fix 1==
The problem flow:
1 - The module is dynamically imported, this creates a cache entry with no
module and no module_promise, and starts an async fetch.

2 - Before dynamicModuleSourceCallback fires (from step 1 above), the same
module is imported as a child of a call graph, i.e. via resolveModuleCallback.
Here the module is compiled, but never evaluated (we only evaluate the root
module). This is where things start to go sour. Our cache entry now has a
module, but no module_promise.

3 - The async fetch completes and calls dynamicModuleSourceCallback which call
Context.module. This returns early (the module is already cached thanks to
step 2). But it then calls resolveDynamicModule which (a) has a module and (b)
no module_promise.

Our fix works because, if Context.module finds the cached module (from step 2),
it now also checks for the module_promise. If it doesn't find it, it evaluates
the module (which sets it).

I've since expanded the code to handle more intermediary states.

The original PR had:

if (gop.value_ptr.module_promise == null) {
    const mod = local.toLocal(cache_mod);
    if (mod.getStatus() == .kInstantiated) {
        return self.evaluateModule(want_result, mod, url, true);
    }
}

But now the code is:

if (gop.value_ptr.module_promise == null) {
    const mod = local.toLocal(cache_mod);
    if (mod.getStatus() == .kUninstantiated and try mod.instantiate(resolveModuleCallback) == false) {
        return error.ModuleInstantiationError;
    }
    return self.evaluateModule(want_result, mod, url, true);
}

It seems that v8 handles double-instantiation and double-evaluations safely.

Handle more partial-load states.
Handle more partial-load states + fix possible dangling pointer.

==Fix 2==
We were using `gop` after potentially writing to the map (via a nested call to
mod.evaluate()). We now re-fetch the map entry to be able to safely write to it

src/browser/js/Context.zig

@@ -324,10 +324,22 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local
             if (gop.found_existing) {
                 if (gop.value_ptr.module) |cache_mod| {
                     if (gop.value_ptr.module_promise == null) {
+                        // This an usual case, but it can happen if a module is
+                        // first asynchronously requested and then synchronously
+                        // requested as a child of some root import. In that case,
+                        // the module may not be instantiated yet (so we have to
+                        // do that). It might not be evaluated yet. So we have
+                        // to do that too. Evaluation is particularly important
+                        // as it sets up our cache entry's module_promise.
+                        // It appears that v8 handles potential double-instantiated
+                        // and double-evaluated modules safely. The 2nd instantiation
+                        // is a no-op, and the second evaluation returns the same
+                        // promise.
                         const mod = local.toLocal(cache_mod);
-                        if (mod.getStatus() == .kInstantiated) {
+                        if (mod.getStatus() == .kUninstantiated and try mod.instantiate(resolveModuleCallback) == false) {
-                            return self.evaluateModule(want_result, mod, url, true);
+                            return error.ModuleInstantiationError;
                         }
+                        return self.evaluateModule(want_result, mod, url, true);
                     }
                     return if (comptime want_result) gop.value_ptr.* else {};
                 }
@@ -691,6 +703,9 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8, referrer: []c
         return promise;
     }
 
+    // we might update the map, so we might need to re-fetch this.
+    var entry = gop.value_ptr;
+
     // So we have a module, but no async resolver. This can only
     // happen if the module was first synchronously loaded (Does that
     // ever even happen?!) You'd think we can just return the module
@@ -703,7 +718,7 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8, referrer: []c
 
     // If the module hasn't been evaluated yet (it was only instantiated
     // as a static import dependency), we need to evaluate it now.
-    if (gop.value_ptr.module_promise == null) {
+    if (entry.module_promise == null) {
         const mod = local.toLocal(gop.value_ptr.module.?);
         const status = mod.getStatus();
         if (status == .kEvaluated or status == .kEvaluating) {
@@ -712,7 +727,7 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8, referrer: []c
             const module_resolver = local.createPromiseResolver();
             module_resolver.resolve("resolve module", mod.getModuleNamespace());
             _ = try module_resolver.persist();
-            gop.value_ptr.module_promise = try module_resolver.promise().persist();
+            entry.module_promise = try module_resolver.promise().persist();
         } else {
             // the module was loaded, but not evaluated, we _have_ to evaluate it now
             const evaluated = mod.evaluate() catch {
@@ -723,18 +738,20 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8, referrer: []c
                 return promise;
             };
             lp.assert(evaluated.isPromise(), "Context._dynamicModuleCallback non-promise", .{});
-            gop.value_ptr.module_promise = try evaluated.toPromise().persist();
+            // mod.evaluate can invalidate or gop
+            entry = self.module_cache.getPtr(specifier).?;
+            entry.module_promise = try evaluated.toPromise().persist();
         }
     }
 
     // like before, we want to set this up so that if anything else
     // tries to load this module, it can just return our promise
     // since we're going to be doing all the work.
-    gop.value_ptr.resolver_promise = try promise.persist();
+    entry.resolver_promise = try promise.persist();
 
     // But we can skip direclty to `resolveDynamicModule` which is
     // what the above callback will eventually do.
-    self.resolveDynamicModule(state, gop.value_ptr.*, local);
+    self.resolveDynamicModule(state, entry.*, local);
     return promise;
 }