VM: preserve dependency code after failed module compilation

Commit fd54d52d freed all bytecode added after the parent module's
compilation checkpoint, including successfully compiled dependencies.
Those dependencies remain cached, so a later import in the same VM
executes freed bytecode.  This is reachable in the interactive shell
and in embedders that continue after a compilation error.

The fix is to remove eager bytecode cleanup while retaining removal of
the failed module from the cache so subsequent loads can retry.  Keep
the removed module and its name in the VM pool because cyclic
dependencies and code metadata can still reference them.  These
allocations are released when the VM is destroyed.

Add a shell regression test that retries a failed parent import and then
executes its cached dependency.

diff --git a/src/njs_generator.c b/src/njs_generator.c
index 3d8a3efa50f9855abd062beb26ba8d86b8713ddd..a3215bcc8e791cf4e36bd8e16b400dda450de567 100644 (file)
--- a/src/njs_generator.c
+++ b/src/njs_generator.c
@@ -5142,29 +5142,6 @@ njs_generate_scope(njs_vm_t *vm, njs_generator_t *generator,
 }
 
 
-void
-njs_generator_cleanup(njs_vm_t *vm, njs_uint_t code_index)
-{
-    njs_vm_code_t  *code;
-
-    if (vm->codes == NULL) {
-        return;
-    }
-
-    while (vm->codes->items > code_index) {
-        code = njs_arr_remove_last(vm->codes);
-
-        if (code->start != NULL) {
-            njs_mp_free(vm->mem_pool, code->start);
-        }
-
-        if (code->lines != NULL) {
-            njs_arr_destroy(code->lines);
-        }
-    }
-}
-
-
 static njs_int_t
 njs_generate_scope_end(njs_vm_t *vm, njs_generator_t *generator,
     njs_parser_node_t *node)

diff --git a/src/njs_generator.h b/src/njs_generator.h
index ae527c4b296ac6a7a5db72cbba917b9858c0545c..689286aadb940d427c50608b1103d6669997cf38 100644 (file)
--- a/src/njs_generator.h
+++ b/src/njs_generator.h
@@ -46,7 +46,6 @@ njs_int_t njs_generator_init(njs_generator_t *generator, njs_mp_t *mem_pool,
     njs_str_t *file, njs_int_t depth, njs_bool_t runtime);
 njs_vm_code_t *njs_generate_scope(njs_vm_t *vm, njs_generator_t *generator,
     njs_parser_scope_t *scope, const njs_str_t *name);
-void njs_generator_cleanup(njs_vm_t *vm, njs_uint_t code_index);
 njs_vm_code_t *njs_lookup_code(njs_vm_t *vm, u_char *pc);
 uint32_t njs_lookup_line(njs_arr_t *lines, uint32_t offset);
 

diff --git a/src/njs_module.c b/src/njs_module.c
index 6b968884da3cf20d09ef4a4860e86a4ec2c900b2..e03cb69d3ef904b226d22e595b49a166b1784d16 100644 (file)
--- a/src/njs_module.c
+++ b/src/njs_module.c
@@ -151,8 +151,7 @@ njs_module_remove(njs_vm_t *vm, njs_mod_t *module)
 
     (void) njs_flathsh_delete(&vm->shared->modules_hash, &fhq);
 
-    njs_mp_free(vm->mem_pool, module->name.start);
-    njs_mp_free(vm->mem_pool, module);
+    /* Compiled dependencies may still reference this module and its name. */
 }
 
 

diff --git a/src/njs_vm.c b/src/njs_vm.c
index 188d85442dda4f58c3830f407f23c41004da57e5..ce9b0ef6762eec73c37842595a65688aa30627e4 100644 (file)
--- a/src/njs_vm.c
+++ b/src/njs_vm.c
@@ -326,7 +326,6 @@ njs_vm_compile_module(njs_vm_t *vm, njs_str_t *name, u_char **start,
     u_char *end)
 {
     njs_int_t              ret;
-    njs_uint_t             code_index;
     njs_mod_t              *module;
     njs_parser_t           parser;
     njs_vm_code_t          *code;
@@ -335,7 +334,6 @@ njs_vm_compile_module(njs_vm_t *vm, njs_str_t *name, u_char **start,
     njs_function_lambda_t  *lambda;
 
     parser.mem_pool = NULL;
-    code_index = (vm->codes != NULL) ? vm->codes->items : 0;
 
     module = njs_module_find(vm, name, 1);
     if (module != NULL) {
@@ -397,7 +395,6 @@ failed:
         njs_parser_destroy(&parser);
     }
 
-    njs_generator_cleanup(vm, code_index);
     njs_module_remove(vm, module);
 
     return NULL;

diff --git a/test/js/module/compile_exception.js b/test/js/module/compile_exception.js
new file mode 100644 (file)
index 0000000..0fae8ef
--- /dev/null
+++ b/test/js/module/compile_exception.js
@@ -0,0 +1,3 @@
+import name from './name.js';
+export default name;
+@

diff --git a/test/shell_test.exp b/test/shell_test.exp
index 000ae63bc2345ce904ea33368c708a768b4dd739..889b5d56e14a71ce17f81879fc7161568735871a 100644 (file)
--- a/test/shell_test.exp
+++ b/test/shell_test.exp
@@ -448,6 +448,15 @@ njs_test {
     {"42\r\n" "42\r\n>> " "unhandled promise rejection"}
 }
 
+njs_test {
+    {"import broken from './test/js/module/compile_exception.js';\r\n"
+     "SyntaxError:*\r\n*>> "}
+    {"import broken from './test/js/module/compile_exception.js';\r\n"
+     "SyntaxError:*\r\n*>> "}
+    {"import name from './test/js/module/name.js'; name;\r\n"
+     "'name'\r\n>> "}
+} "-n njs"
+
 # CLI OPTIONS
 
 # help