Merge branch 'main' into dylan/fix-fs-abort-signal-leak

src/bun.js/bindings/BunObject.cpp

@@ -550,14 +550,20 @@ JSC_DEFINE_HOST_FUNCTION(functionPathToFileURL, (JSC::JSGlobalObject * lexicalGl
     auto throwScope = DECLARE_THROW_SCOPE(vm);
     auto pathValue = callFrame->argument(0);
 
-    WTF::String pathString = pathValue.toWTFString(lexicalGlobalObject);
-    RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode({}));
+    JSValue jsValue;
 
-    pathString = pathResolveWTFString(lexicalGlobalObject, pathString);
+    {
+        WTF::String pathString = pathValue.toWTFString(lexicalGlobalObject);
+        RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode({}));
+        pathString = pathResolveWTFString(lexicalGlobalObject, pathString);
+
+        auto fileURL = WTF::URL::fileURLWithFileSystemPath(pathString);
+        auto object = WebCore::DOMURL::create(fileURL.string(), String());
+        jsValue = WebCore::toJSNewlyCreated<IDLInterface<DOMURL>>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object));
+    }
 
-    auto fileURL = WTF::URL::fileURLWithFileSystemPath(pathString);
-    auto object = WebCore::DOMURL::create(fileURL.string(), String());
-    auto jsValue = WebCore::toJSNewlyCreated<IDLInterface<DOMURL>>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object));
+    auto* jsDOMURL = jsCast<JSDOMURL*>(jsValue.asCell());
+    vm.heap.reportExtraMemoryAllocated(jsDOMURL, jsDOMURL->wrapped().memoryCostForGC());
     RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(jsValue));
 }
 

src/bun.js/bindings/BunString.cpp

@@ -328,16 +328,15 @@ extern "C" BunString BunString__fromUTF8(const char* bytes, size_t length)
             return { .tag = BunStringTag::Dead };
         }
         RELEASE_ASSERT(simdutf::convert_utf8_to_utf16(bytes, length, ptr.data()) == u16Length);
-        impl->ref();
         return { BunStringTag::WTFStringImpl, { .wtf = impl.leakRef() } };
     }
 
     auto str = WTF::String::fromUTF8ReplacingInvalidSequences(std::span { reinterpret_cast<const LChar*>(bytes), length });
     if (UNLIKELY(str.isNull())) {
         return { .tag = BunStringTag::Dead };
     }
-    str.impl()->ref();
-    return Bun::toString(str);
+    auto impl = str.releaseImpl();
+    return Bun::toString(impl.leakRef());
 }
 
 extern "C" BunString BunString__fromLatin1(const char* bytes, size_t length)
@@ -481,6 +480,8 @@ extern "C" JSC__JSValue BunString__toJSDOMURL(JSC::JSGlobalObject* lexicalGlobal
 
     auto object = WebCore::DOMURL::create(str, String());
     auto jsValue = WebCore::toJSNewlyCreated<WebCore::IDLInterface<WebCore::DOMURL>>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object));
+    auto* jsDOMURL = jsCast<WebCore::JSDOMURL*>(jsValue.asCell());
+    vm.heap.reportExtraMemoryAllocated(jsDOMURL, jsDOMURL->wrapped().memoryCostForGC());
     RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(jsValue));
 }
 
@@ -663,6 +664,34 @@ WTF::String BunString::toWTFString(ZeroCopyTag) const
     return WTF::String();
 }
 
+WTF::String BunString::transferToWTFString()
+{
+    if (this->tag == BunStringTag::ZigString) {
+        if (Zig::isTaggedUTF8Ptr(this->impl.zig.ptr)) {
+            auto str = Zig::toStringCopy(this->impl.zig);
+            *this = Zig::BunStringEmpty;
+            return str;
+        } else {
+            auto str = Zig::toString(this->impl.zig);
+            *this = Zig::BunStringEmpty;
+            return str;
+        }
+    } else if (this->tag == BunStringTag::StaticZigString) {
+        auto str = Zig::toStringStatic(this->impl.zig);
+        *this = Zig::BunStringEmpty;
+        return str;
+    } else if (this->tag == BunStringTag::WTFStringImpl) {
+        ASSERT(this->impl.wtf->refCount() > 0 && !this->impl.wtf->isEmpty());
+
+        auto str = WTF::String(this->impl.wtf);
+        this->impl.wtf->deref();
+        *this = Zig::BunStringEmpty;
+        return str;
+    }
+
+    return WTF::String();
+}
+
 extern "C" BunString BunString__createExternalGloballyAllocatedLatin1(
     const LChar* bytes,
     size_t length)

src/bun.js/bindings/DOMURL.cpp

@@ -57,6 +57,7 @@ static inline String redact(const String& input)
 
 inline DOMURL::DOMURL(URL&& completeURL)
     : m_url(WTFMove(completeURL))
+    , m_initialURLCostForGC(std::min(static_cast<short>(m_url.string().impl()->costDuringGC()), std::numeric_limits<short>::max()))
 {
     ASSERT(m_url.isValid());
 }

src/bun.js/bindings/DOMURL.h

@@ -65,6 +65,10 @@ class DOMURL final : public RefCounted<DOMURL>, public CanMakeWeakPtr<DOMURL>, p
     {
         return sizeof(DOMURL) + m_url.string().sizeInBytes();
     }
+    size_t memoryCostForGC() const
+    {
+        return sizeof(DOMURL) + m_initialURLCostForGC;
+    }
 
 private:
     static ExceptionOr<Ref<DOMURL>> create(const String& url, const URL& base);
@@ -75,6 +79,7 @@ class DOMURL final : public RefCounted<DOMURL>, public CanMakeWeakPtr<DOMURL>, p
 
     URL m_url;
     RefPtr<URLSearchParams> m_searchParams;
+    short m_initialURLCostForGC { 0 };
 };
 
 } // namespace WebCore

src/bun.js/bindings/JSBuffer.cpp

@@ -1618,11 +1618,12 @@ static inline JSC::EncodedJSValue jsBufferToString(JSC::VM& vm, JSC::JSGlobalObj
     case WebCore::BufferEncodingType::base64url:
     case WebCore::BufferEncodingType::hex: {
         ret = Bun__encoding__toString(reinterpret_cast<const unsigned char*>(castedThis->vector()) + offset, length, lexicalGlobalObject, static_cast<uint8_t>(encoding));
+        RETURN_IF_EXCEPTION(scope, {});
         break;
     }
     default: {
         throwTypeError(lexicalGlobalObject, scope, "Unsupported encoding? This shouldn't happen"_s);
-        break;
+        return {};
     }
     }
 

src/bun.js/bindings/PathInlines.h

@@ -58,11 +58,11 @@ ALWAYS_INLINE bool isAbsolutePath(WTF::String input)
 extern "C" BunString ResolvePath__joinAbsStringBufCurrentPlatformBunString(JSC::JSGlobalObject*, BunString);
 
 /// CWD is determined by the global object's current cwd.
-ALWAYS_INLINE WTF::String pathResolveWTFString(JSC::JSGlobalObject* globalToGetCwdFrom, WTF::String input)
+ALWAYS_INLINE WTF::String pathResolveWTFString(JSC::JSGlobalObject* globalToGetCwdFrom, const WTF::String& input)
 {
     if (isAbsolutePath(input))
         return input;
     BunString in = Bun::toString(input);
     BunString out = ResolvePath__joinAbsStringBufCurrentPlatformBunString(globalToGetCwdFrom, in);
-    return out.toWTFString();
+    return out.transferToWTFString();
 }

src/bun.js/bindings/bindings.zig

@@ -5330,7 +5330,7 @@ pub const JSValue = enum(i64) {
         return getOwnTruthy(this, global, property);
     }
 
-    pub fn truthyPropertyValue(prop: JSValue) ?JSValue {
+    fn truthyPropertyValue(prop: JSValue) ?JSValue {
         return switch (prop) {
             .zero => unreachable,
 

src/bun.js/bindings/headers-handwritten.h

@@ -69,6 +69,8 @@ typedef struct BunString {
     // It's only truly zero-copy if it was already a WTFStringImpl (which it is if it came from JS and we didn't use ZigString)
     WTF::String toWTFString(ZeroCopyTag) const;
 
+    WTF::String transferToWTFString();
+
     // This one usually will clone the raw bytes.
     WTF::String toWTFString() const;
 

src/bun.js/bindings/webcore/JSDOMURL.cpp

@@ -171,6 +171,8 @@ template<> EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSDOMURLDOMConstructor::const
         RETURN_IF_EXCEPTION(throwScope, {});
     setSubclassStructureIfNeeded<DOMURL>(lexicalGlobalObject, callFrame, asObject(jsValue));
     RETURN_IF_EXCEPTION(throwScope, {});
+    auto* jsDOMURL = jsCast<JSDOMURL*>(jsValue.asCell());
+    vm.heap.reportExtraMemoryAllocated(jsDOMURL, jsDOMURL->wrapped().memoryCostForGC());
     return JSValue::encode(jsValue);
 }
 JSC_ANNOTATE_HOST_FUNCTION(JSDOMURLDOMConstructorConstruct, JSDOMURLDOMConstructor::construct);
@@ -787,6 +789,7 @@ void JSDOMURL::visitChildrenImpl(JSCell* cell, Visitor& visitor)
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     Base::visitChildren(thisObject, visitor);
     visitor.append(thisObject->m_searchParams);
+    visitor.reportExtraMemoryVisited(thisObject->protectedWrapped()->memoryCostForGC());
 }
 
 DEFINE_VISIT_CHILDREN(JSDOMURL);

src/bun.js/bindings/webcore/WebSocket.cpp

@@ -1464,9 +1464,9 @@ extern "C" void WebSocket__didAbruptClose(WebCore::WebSocket* webSocket, int32_t
 {
     webSocket->didFailWithErrorCode(errorCode);
 }
-extern "C" void WebSocket__didClose(WebCore::WebSocket* webSocket, uint16_t errorCode, const BunString* reason)
+extern "C" void WebSocket__didClose(WebCore::WebSocket* webSocket, uint16_t errorCode, BunString* reason)
 {
-    WTF::String wtf_reason = reason->toWTFString(BunString::ZeroCopy);
+    WTF::String wtf_reason = reason->transferToWTFString();
     webSocket->didClose(0, errorCode, WTFMove(wtf_reason));
 }
 

src/bun.js/webcore/encoding.zig

@@ -1153,7 +1153,14 @@ pub const Encoder = struct {
                 return str;
             },
             .buffer, .utf8 => {
-                return bun.String.createUTF8(input);
+                const converted = strings.toUTF16Alloc(bun.default_allocator, input, false, false) catch return bun.String.dead;
+                if (converted) |utf16| {
+                    return bun.String.createExternalGloballyAllocated(.utf16, utf16);
+                }
+
+                // If we get here, it means we can safely assume the string is 100% ASCII characters
+                // For this, we rely on WebKit to manage the memory.
+                return bun.String.createLatin1(input);
             },
             .ucs2, .utf16le => {
                 // Avoid incomplete characters
@@ -1176,16 +1183,17 @@ pub const Encoder = struct {
             },
 
             .base64url => {
-                const str, const chars = bun.String.createUninitialized(.latin1, bun.base64.urlSafeEncodeLen(input));
-                _ = bun.base64.encodeURLSafe(chars, input);
-                return str;
+                const to_len = bun.base64.urlSafeEncodeLen(input);
+                const to = bun.default_allocator.alloc(u8, to_len) catch return bun.String.dead;
+                const wrote = bun.base64.encodeURLSafe(to, input);
+                return bun.String.createExternalGloballyAllocated(.latin1, to[0..wrote]);
             },
 
             .base64 => {
                 const to_len = bun.base64.encodeLen(input);
-                const str, const chars = bun.String.createUninitialized(.latin1, to_len);
-                _ = bun.base64.encode(chars, input);
-                return str;
+                const to = bun.default_allocator.alloc(u8, to_len) catch return bun.String.dead;
+                const wrote = bun.base64.encode(to, input);
+                return bun.String.createExternalGloballyAllocated(.latin1, to[0..wrote]);
             },
         }
     }

src/bun.js/webcore/request.zig

@@ -490,6 +490,7 @@ pub const Request = struct {
                     const href = bun.JSC.URL.hrefFromString(this.url);
                     // TODO: what is the right thing to do for invalid URLS?
                     if (!href.isEmpty()) {
+                        this.url.deref();
                         this.url = href;
                     }
 

src/http/websocket_http_client.zig

@@ -161,7 +161,7 @@ const CppWebSocket = opaque {
         defer loop.exit();
         WebSocket__didAbruptClose(this, reason);
     }
-    pub fn didClose(this: *CppWebSocket, code: u16, reason: *const bun.String) void {
+    pub fn didClose(this: *CppWebSocket, code: u16, reason: *bun.String) void {
         const loop = JSC.VirtualMachine.get().eventLoop();
         loop.enter();
         defer loop.exit();
@@ -1846,7 +1846,7 @@ pub fn NewWebSocketClient(comptime ssl: bool) type {
             this.deref();
         }
 
-        fn dispatchClose(this: *WebSocket, code: u16, reason: *const bun.String) void {
+        fn dispatchClose(this: *WebSocket, code: u16, reason: *bun.String) void {
             var out = this.outgoing_websocket orelse return;
             this.poll_ref.unref(this.globalThis.bunVM());
             JSC.markBinding(@src());

src/resolver/resolver.zig

@@ -3332,7 +3332,7 @@ pub const Resolver = struct {
     pub export fn Resolver__propForRequireMainPaths(globalThis: *bun.JSC.JSGlobalObject) callconv(.C) JSC.JSValue {
         bun.JSC.markBinding(@src());
 
-        const in_str = bun.String.createUTF8(".");
+        const in_str = bun.String.init(".");
         const r = &globalThis.bunVM().transpiler.resolver;
         return nodeModulePathsJSValue(r, in_str, globalThis);
     }

test/js/bun/http/req-url-leak.test.ts

@@ -0,0 +1,47 @@
+import { test, expect } from "bun:test";
+import { bunExe, bunEnv } from "harness";
+import path from "path";
+test("req.url doesn't leak memory", async () => {
+  const { promise, resolve } = Promise.withResolvers();
+  await using process = Bun.spawn({
+    cmd: [bunExe(), path.join(import.meta.dir, "req-url-leak-fixture.js")],
+    env: bunEnv,
+    stdout: "inherit",
+    stderr: "inherit",
+    stdin: "inherit",
+    ipc(message, child) {
+      if (message.url) {
+        resolve(message.url);
+      }
+    },
+  });
+
+  const baseURL = await promise;
+
+  const url = new URL(Buffer.alloc(1024 * 15, "Z").toString(), baseURL);
+
+  let maxRSS = 0;
+
+  for (let i = 0; i < 512; i++) {
+    const batchSize = 64;
+    const promises = [];
+    for (let j = 0; j < batchSize; j++) {
+      promises.push(
+        fetch(url)
+          .then(r => r.text())
+          .then(rssText => {
+            const rss = parseFloat(rssText);
+            if (Number.isSafeInteger(rss)) {
+              maxRSS = Math.max(maxRSS, rss);
+            }
+          }),
+      );
+    }
+    await Promise.all(promises);
+  }
+
+  console.log("RSS", maxRSS);
+
+  // 557 MB on Bun 1.2
+  expect(maxRSS).toBeLessThan(1024 * 1024 * 256);
+}, 10_000);

test/js/node/url/pathToFileURL.test.ts

@@ -0,0 +1,5 @@
+import { test, expect } from "bun:test";
+import path from "path";
+test("pathToFileURL doesn't leak memory", () => {
+  expect([path.join(import.meta.dir, "pathToFileURL-leak-fixture.js")]).toRun();
+});