Fixed module importing using require().

Previously, require() did not make a mutable copy of imported module.
As a result, cloned VMs could change a shared module object making
module.object->hash inconsistent.

Before 04d7a5d93ae6, the problem manifested itself only in "PROP GET"
operations, for example, using "require('fs').renameSync", because
njs_value_property() makes a mutable copy of shared methods in
module.object->hash.

After 04d7a5d93ae6, "METHOD CALL" operations, such as
"require('fs').renameSync()", now have the same problem as in "PROP
GET".

Importing modules using "import" statement is not affected, because
for it "OBJECT COPY" instruction is generated, which makes a mutable
copy of imported module object.

The fix is to make a mutable copy of a shared module in require().

This closes #206 issue on Github.

diff --git a/src/njs_module.c b/src/njs_module.c
index 552770a32185e275041fd5ea28b8bdb1b3b4edf9..750c3ba3bdcc19acf65cf5ca757465ee4eef0d62 100644 (file)
--- a/src/njs_module.c
+++ b/src/njs_module.c
@@ -503,9 +503,10 @@ njs_module_insert(njs_vm_t *vm, njs_module_t *module)
 
 
 njs_int_t
-njs_module_require(njs_vm_t *vm, njs_value_t *args,
-    njs_uint_t nargs, njs_index_t unused)
+njs_module_require(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
+    njs_index_t unused)
 {
+    njs_object_t        *object;
     njs_module_t        *module;
     njs_lvlhsh_query_t  lhq;
 
@@ -520,9 +521,18 @@ njs_module_require(njs_vm_t *vm, njs_value_t *args,
 
     if (njs_lvlhsh_find(&vm->modules_hash, &lhq) == NJS_OK) {
         module = lhq.value;
-        module->object.__proto__ = &vm->prototypes[NJS_PROTOTYPE_OBJECT].object;
 
-        njs_set_object(&vm->retval, &module->object);
+        object = njs_mp_alloc(vm->mem_pool, sizeof(njs_object_t));
+        if (njs_slow_path(object == NULL)) {
+            njs_memory_error(vm);
+            return NJS_ERROR;
+        }
+
+        *object = module->object;
+        object->__proto__ = &vm->prototypes[NJS_PROTOTYPE_OBJECT].object;
+        object->shared = 0;
+
+        njs_set_object(&vm->retval, object);
 
         return NJS_OK;
     }

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index aadb8f338e0d68605642a7fc623e10d0a313f932..c28a7d84ffc7ed23e02c6bbef9d3d02946d10671 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -13489,6 +13489,22 @@ static njs_unit_test_t  njs_module_test[] =
 };
 
 
+static njs_unit_test_t  njs_shared_test[] =
+{
+    { njs_str("var cr = require('crypto'); cr.createHash"),
+      njs_str("[object Function]") },
+
+    { njs_str("var cr = require('crypto'); cr.createHash('md5')"),
+      njs_str("[object Hash]") },
+
+    { njs_str("import cr from 'crypto'; cr.createHash"),
+      njs_str("[object Function]") },
+
+    { njs_str("import cr from 'crypto'; cr.createHash('md5')"),
+      njs_str("[object Hash]") },
+};
+
+
 static njs_unit_test_t  njs_tz_test[] =
 {
      { njs_str("var d = new Date(1); d = d + ''; d.slice(0, 33)"),
@@ -14278,6 +14294,7 @@ typedef struct {
     njs_bool_t  disassemble;
     njs_bool_t  verbose;
     njs_bool_t  module;
+    njs_uint_t  repeat;
 } njs_opts_t;
 
 
@@ -14308,7 +14325,7 @@ njs_unit_test(njs_unit_test_t tests[], size_t num, const char *name,
     njs_vm_t      *vm, *nvm;
     njs_int_t     ret;
     njs_str_t     s;
-    njs_uint_t    i;
+    njs_uint_t    i, repeat;
     njs_stat_t    prev;
     njs_bool_t    success;
     njs_vm_opt_t  options;
@@ -14350,13 +14367,21 @@ njs_unit_test(njs_unit_test_t tests[], size_t num, const char *name,
                 njs_disassembler(vm);
             }
 
-            nvm = njs_vm_clone(vm, NULL);
-            if (nvm == NULL) {
-                njs_printf("njs_vm_clone() failed\n");
-                goto done;
-            }
+            repeat = opts->repeat;
 
-            ret = njs_vm_start(nvm);
+            do {
+                if (nvm != NULL) {
+                    njs_vm_destroy(nvm);
+                }
+
+                nvm = njs_vm_clone(vm, NULL);
+                if (nvm == NULL) {
+                    njs_printf("njs_vm_clone() failed\n");
+                    goto done;
+                }
+
+                ret = njs_vm_start(nvm);
+            } while (--repeat != 0);
 
             if (njs_vm_retval_string(nvm, &s) != NJS_OK) {
                 njs_printf("njs_vm_retval_string() failed\n");
@@ -14844,6 +14869,8 @@ main(int argc, char **argv)
 
     njs_memzero(&stat, sizeof(njs_stat_t));
 
+    opts.repeat = 1;
+
     ret = njs_unit_test(njs_test, njs_nitems(njs_test), "script tests",
                         &opts, &stat);
     if (ret != NJS_OK) {
@@ -14878,6 +14905,15 @@ main(int argc, char **argv)
         return ret;
     }
 
+    opts.module = 0;
+    opts.repeat = 128;
+
+    ret = njs_unit_test(njs_shared_test, njs_nitems(njs_shared_test),
+                        "shared tests", &opts, &stat);
+    if (ret != NJS_OK) {
+        return ret;
+    }
+
     njs_printf("TOTAL: %s [%ui/%ui]\n", stat.failed ? "FAILED" : "PASSED",
                stat.passed, stat.passed + stat.failed);