Make readdir() async, fix crash in large directory trees (oven-sh#3838)

* Fix unsafe GC behavior on large arrays returned by fs * Fix crash in large arrays of strings * async readdir * Add tests for large number of files returned by readdir * Move this down * Fix encoding edgecase in path.join * Async stat & lstat * add test --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
trnxdev · Aug 9, 2023 · 4476b2f · 4476b2f
1 parent 504f506
commit 4476b2f
Show file tree

Hide file tree

Showing 10 changed files with 461 additions and 232 deletions.
diff --git a/src/bun.js/base.zig b/src/bun.js/base.zig
@@ -266,58 +266,21 @@ pub const To = struct {
 
                     // Recursion can stack overflow here
                     if (comptime std.meta.trait.isSlice(Type)) {
-                        const Child = std.meta.Child(Type);
-
-                        const prefill = 32;
-                        if (value.len <= prefill) {
-                            var array: [prefill]JSC.C.JSValueRef = undefined;
-                            var i: u8 = 0;
-                            const len = @min(@as(u8, @intCast(value.len)), prefill);
-                            while (i < len and exception.* == null) : (i += 1) {
-                                array[i] = if (comptime Child == JSC.C.JSValueRef)
-                                    value[i]
-                                else
-                                    To.JS.withType(Child, value[i], context, exception);
-                            }
-
-                            if (exception.* != null) {
-                                return null;
-                            }
-
-                            // TODO: this function copies to a MarkedArgumentsBuffer
-                            // That copy is unnecessary.
-                            const obj = JSC.C.JSObjectMakeArray(context, len, &array, exception);
-
-                            if (exception.* != null) {
-                                return null;
-                            }
-                            return obj;
-                        }
-
-                        {
-                            var array = bun.default_allocator.alloc(JSC.C.JSValueRef, value.len) catch unreachable;
-                            defer bun.default_allocator.free(array);
-                            var i: usize = 0;
-                            while (i < value.len and exception.* == null) : (i += 1) {
-                                array[i] = if (comptime Child == JSC.C.JSValueRef)
-                                    value[i]
-                                else
-                                    To.JS.withType(Child, value[i], context, exception);
-                            }
+                        const Child = comptime std.meta.Child(Type);
 
-                            if (exception.* != null) {
-                                return null;
-                            }
+                        var array = JSC.JSValue.createEmptyArray(context, value.len);
+                        for (value, 0..) |item, i| {
+                            array.putIndex(
+                                context,
+                                @truncate(i),
+                                JSC.JSValue.c(To.JS.withType(Child, item, context, exception)),
+                            );
 
-                            // TODO: this function copies to a MarkedArgumentsBuffer
-                            // That copy is unnecessary.
-                            const obj = JSC.C.JSObjectMakeArray(context, value.len, array.ptr, exception);
                             if (exception.* != null) {
                                 return null;
                             }
-
-                            return obj;
                         }
+                        return array.asObjectRef();
                     }
 
                     if (comptime std.meta.trait.isZigString(Type)) {

diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp
@@ -256,31 +256,47 @@ extern "C" EncodedJSValue BunString__createArray(
     auto& vm = globalObject->vm();
     auto throwScope = DECLARE_THROW_SCOPE(vm);
 
-    // We must do this or Bun.gc(true) in a loop creating large arrays of strings will crash due to GC'ing.
-    MarkedArgumentBuffer arguments;
-    JSC::ObjectInitializationScope scope(vm);
-    GCDeferralContext context(vm);
-
-    arguments.fill(length, [&](JSC::JSValue* value) {
-        const BunString* end = ptr + length;
-        while (ptr != end) {
-            *value++ = Bun::toJS(globalObject, *ptr++);
-        }
-    });
+    if (length < 64) {
+        // We must do this or Bun.gc(true) in a loop creating large arrays of strings will crash due to GC'ing.
+        MarkedArgumentBuffer arguments;
+
+        arguments.fill(length, [&](JSC::JSValue* value) {
+            const BunString* end = ptr + length;
+            while (ptr != end) {
+                *value++ = Bun::toJS(globalObject, *ptr++);
+            }
+        });
+
+        JSC::ObjectInitializationScope scope(vm);
+        GCDeferralContext context(vm);
 
-    if (JSC::JSArray* array = JSC::JSArray::tryCreateUninitializedRestricted(
+        JSC::JSArray* array = JSC::JSArray::tryCreateUninitializedRestricted(
             scope,
             globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous),
-            length)) {
+            length);
+
+        if (array) {
+            for (size_t i = 0; i < length; ++i) {
+                array->initializeIndex(scope, i, arguments.at(i));
+            }
+            return JSValue::encode(array);
+        }
+
+        JSC::throwOutOfMemoryError(globalObject, throwScope);
+        RELEASE_AND_RETURN(throwScope, JSValue::encode(JSC::JSValue()));
+    } else {
+        JSC::JSArray* array = constructEmptyArray(globalObject, nullptr, length);
+        if (!array) {
+            JSC::throwOutOfMemoryError(globalObject, throwScope);
+            RELEASE_AND_RETURN(throwScope, JSValue::encode(JSC::JSValue()));
+        }
 
         for (size_t i = 0; i < length; ++i) {
-            array->initializeIndex(scope, i, arguments.at(i));
+            array->putDirectIndex(globalObject, i, Bun::toJS(globalObject, *ptr++));
         }
+
         return JSValue::encode(array);
     }
-
-    JSC::throwOutOfMemoryError(globalObject, throwScope);
-    RELEASE_AND_RETURN(throwScope, JSValue::encode(JSC::JSValue()));
 }
 
 extern "C" void BunString__toWTFString(BunString* bunString)