diff --git a/src/bun.js/bindings/BunObject.cpp b/src/bun.js/bindings/BunObject.cpp index 8290a280d35910..4f5daf940e3956 100644 --- a/src/bun.js/bindings/BunObject.cpp +++ b/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>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object)); + } - auto fileURL = WTF::URL::fileURLWithFileSystemPath(pathString); - auto object = WebCore::DOMURL::create(fileURL.string(), String()); - auto jsValue = WebCore::toJSNewlyCreated>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object)); + auto* jsDOMURL = jsCast(jsValue.asCell()); + vm.heap.reportExtraMemoryAllocated(jsDOMURL, jsDOMURL->wrapped().memoryCostForGC()); RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(jsValue)); } diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp index 55ccacc5194794..6a1e1663567880 100644 --- a/src/bun.js/bindings/BunString.cpp +++ b/src/bun.js/bindings/BunString.cpp @@ -328,7 +328,6 @@ 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() } }; } @@ -336,8 +335,8 @@ extern "C" BunString BunString__fromUTF8(const char* bytes, size_t 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>(*lexicalGlobalObject, globalObject, throwScope, WTFMove(object)); + auto* jsDOMURL = jsCast(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) diff --git a/src/bun.js/bindings/DOMURL.cpp b/src/bun.js/bindings/DOMURL.cpp index 7c59c949501e0c..690dbff4aa142c 100644 --- a/src/bun.js/bindings/DOMURL.cpp +++ b/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(m_url.string().impl()->costDuringGC()), std::numeric_limits::max())) { ASSERT(m_url.isValid()); } diff --git a/src/bun.js/bindings/DOMURL.h b/src/bun.js/bindings/DOMURL.h index 56db692140943e..cb4bba6d8b2f35 100644 --- a/src/bun.js/bindings/DOMURL.h +++ b/src/bun.js/bindings/DOMURL.h @@ -65,6 +65,10 @@ class DOMURL final : public RefCounted, public CanMakeWeakPtr, p { return sizeof(DOMURL) + m_url.string().sizeInBytes(); } + size_t memoryCostForGC() const + { + return sizeof(DOMURL) + m_initialURLCostForGC; + } private: static ExceptionOr> create(const String& url, const URL& base); @@ -75,6 +79,7 @@ class DOMURL final : public RefCounted, public CanMakeWeakPtr, p URL m_url; RefPtr m_searchParams; + short m_initialURLCostForGC { 0 }; }; } // namespace WebCore diff --git a/src/bun.js/bindings/JSBuffer.cpp b/src/bun.js/bindings/JSBuffer.cpp index 7a99b8711920c7..e948e736438f28 100644 --- a/src/bun.js/bindings/JSBuffer.cpp +++ b/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(castedThis->vector()) + offset, length, lexicalGlobalObject, static_cast(encoding)); + RETURN_IF_EXCEPTION(scope, {}); break; } default: { throwTypeError(lexicalGlobalObject, scope, "Unsupported encoding? This shouldn't happen"_s); - break; + return {}; } } diff --git a/src/bun.js/bindings/PathInlines.h b/src/bun.js/bindings/PathInlines.h index 363d9269a0d3a2..e0dbf484f4c8cd 100644 --- a/src/bun.js/bindings/PathInlines.h +++ b/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(); } diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 6f7ae454686303..e2ab869419df5b 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/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, diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h index 4da107cbbdf1d9..9a81eda9783d81 100644 --- a/src/bun.js/bindings/headers-handwritten.h +++ b/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; diff --git a/src/bun.js/bindings/webcore/JSDOMURL.cpp b/src/bun.js/bindings/webcore/JSDOMURL.cpp index c190da7c098828..9d7677fb738e33 100755 --- a/src/bun.js/bindings/webcore/JSDOMURL.cpp +++ b/src/bun.js/bindings/webcore/JSDOMURL.cpp @@ -171,6 +171,8 @@ template<> EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSDOMURLDOMConstructor::const RETURN_IF_EXCEPTION(throwScope, {}); setSubclassStructureIfNeeded(lexicalGlobalObject, callFrame, asObject(jsValue)); RETURN_IF_EXCEPTION(throwScope, {}); + auto* jsDOMURL = jsCast(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); diff --git a/src/bun.js/bindings/webcore/WebSocket.cpp b/src/bun.js/bindings/webcore/WebSocket.cpp index b2c8622f943bbb..c35bf6c3e90569 100644 --- a/src/bun.js/bindings/webcore/WebSocket.cpp +++ b/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)); } diff --git a/src/bun.js/webcore/encoding.zig b/src/bun.js/webcore/encoding.zig index cdfbb4893dd964..47d3abaa5236a3 100644 --- a/src/bun.js/webcore/encoding.zig +++ b/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]); }, } } diff --git a/src/http/websocket_http_client.zig b/src/http/websocket_http_client.zig index 734b795835dffc..5b2987f9be903f 100644 --- a/src/http/websocket_http_client.zig +++ b/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()); diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index 17875431c2899b..84a4c127658ffd 100644 --- a/src/resolver/resolver.zig +++ b/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); } diff --git a/test/js/node/url/pathToFileURL-leak-fixture.js b/test/js/node/url/pathToFileURL-leak-fixture.js new file mode 100644 index 00000000000000..bc1557a0476adf --- /dev/null +++ b/test/js/node/url/pathToFileURL-leak-fixture.js @@ -0,0 +1,13 @@ +var longPath = Buffer.alloc(1021, "Z").toString(); +const isDebugBuildOfBun = globalThis?.Bun?.revision?.includes("debug"); +import { pathToFileURL } from "url"; +for (let i = 0; i < 1024 * (isDebugBuildOfBun ? 32 : 256); i++) { + pathToFileURL(longPath); +} +Bun.gc(true); +const rss = (process.memoryUsage.rss() / 1024 / 1024) | 0; +console.log("RSS", rss, "MB"); +if (rss > 250) { + // On macOS, this was 860 MB. + throw new Error("RSS is too high. Must be less than 250MB"); +} diff --git a/test/js/node/url/pathToFileURL.test.ts b/test/js/node/url/pathToFileURL.test.ts new file mode 100644 index 00000000000000..e9e654764c86b2 --- /dev/null +++ b/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(); +});