Fix Array.prototype.sort() with huge sparse arrays

Previously, sort deleted every index from the compacted output position to
the original length.  A sparse array with length near 2^32 therefore caused
billions of property-delete operations and unbounded memory growth.

The fix is to retain the direct cleanup loop for fast arrays and, for slow
arrays, enumerate and delete only existing indexed properties in the range.

The issue was introduced in 1a2a65f7 (0.8.0).

This closes #1095 issue on GitHub.

diff --git a/src/njs_array.c b/src/njs_array.c
index a9ea5143f9def4e09016e0ffb914f827eb1fdaab..aea9c32b6b4e47bc3cbc11bbcf493febc4f94c42 100644 (file)
--- a/src/njs_array.c
+++ b/src/njs_array.c
@@ -2917,8 +2917,11 @@ static njs_int_t
 njs_array_prototype_sort(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused, njs_value_t *retval)
 {
+    double                 idx;
     int64_t                i, nslots, nunds, length;
+    uint32_t               n;
     njs_int_t              ret;
+    njs_array_t            *keys;
     njs_value_t            *this, *comparefn;
     njs_function_t         *compare;
     njs_array_sort_slot_t  *slots;
@@ -2982,11 +2985,35 @@ njs_array_prototype_sort(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         }
     }
 
-    for (; i < length; i++) {
-        ret = njs_value_property_i64_delete(vm, this, i, NULL);
-        if (njs_slow_path(ret == NJS_ERROR)) {
+    if (njs_is_fast_array(this)) {
+        for (; i < length; i++) {
+            ret = njs_value_property_i64_delete(vm, this, i, NULL);
+            if (njs_slow_path(ret == NJS_ERROR)) {
+                goto exception;
+            }
+        }
+
+    } else {
+        keys = njs_array_indices(vm, this);
+        if (njs_slow_path(keys == NULL)) {
             goto exception;
         }
+
+        for (n = 0; n < keys->length; n++) {
+            idx = njs_string_to_index(&keys->start[n]);
+
+            if (idx >= i && idx < length) {
+                ret = njs_value_property_delete(vm, this,
+                                                 keys->start[n].atom_id,
+                                                 NULL, 1);
+                if (njs_slow_path(ret == NJS_ERROR)) {
+                    njs_array_destroy(vm, keys);
+                    goto exception;
+                }
+            }
+        }
+
+        njs_array_destroy(vm, keys);
     }
 
 done:

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index d7137c569bd18135a455f98dfbdd006dc7b9eb22..3c01172d8a34da4bcfeefb37c8c1f606fe918346 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -5193,6 +5193,10 @@ static njs_unit_test_t  njs_test[] =
               "njs.dump(a2)"),
       njs_str("[3,1,<1111110 empty items>,2,4]") },
 
+    { njs_str("var a = []; a[2**32 - 7] = 2; a[2**32 - 2] = 1;"
+              "a.sort(); [a.length, a[0], a[1], Object.keys(a)]"),
+      njs_str("4294967295,1,2,0,1") },
+
     { njs_str("var a = []; a[100000] = 1;"
               "var b = [].concat(a, [,, 5]);"
               "[b.length, Object.keys(b), b[100001], b.sort().length]"),