Fix segfault due to GC and some more helper functions (#1221)

* Fix segfault due to GC and some more helper functions

* fix upon reviews

* add visitChildren

1 files changed, 121 insertions, 50 deletions

diff --git a/src/bun.js/bindings/JSReadableState.cpp b/src/bun.js/bindings/JSReadableState.cpp
index bf1bc1155..cab54f1ce 100644
--- a/src/bun.js/bindings/JSReadableState.cpp
+++ b/src/bun.js/bindings/JSReadableState.cpp
@@ -38,77 +38,70 @@ void JSReadableState::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObj
 {
     Base::finishCreation(vm);
 
-    bool objectMode = false;
-    auto objectModeIdent = JSC::Identifier::fromString(vm, "objectMode"_s);
     if (options != nullptr) {
-        JSC::JSValue objectModeVal = options->getDirect(vm, objectModeIdent);
+        JSC::JSValue objectModeVal = options->getDirect(vm, JSC::Identifier::fromString(vm, "objectMode"_s));
         if (isDuplex && !objectModeVal) {
             objectModeVal = options->getDirect(vm, JSC::Identifier::fromString(vm, "readableObjectMode"_s));
         }
-        if (objectModeVal)
-            objectMode = objectModeVal.toBoolean(globalObject);
+        if (objectModeVal && objectModeVal.toBoolean(globalObject))
+            setBool(JSReadableState::Mask::objectMode, true);
     }
-    putDirect(vm, WTFMove(objectModeIdent), JSC::jsBoolean(objectMode));
 
-    m_highWaterMark = objectMode ? 16 : 16 * 1024;  // default value
+    m_highWaterMark = getBool(JSReadableState::Mask::objectMode) ? 16 : 16 * 1024;  // default value
     if (options != nullptr) {
         int64_t customHightWaterMark = getHighWaterMark(vm, globalObject, isDuplex, options);
         if (customHightWaterMark >= 0)
             m_highWaterMark = customHightWaterMark;
     }
 
-    putDirect(vm, JSC::Identifier::fromString(vm, "buffer"_s), JSBufferList::create(
+    m_buffer.set(vm, this, JSBufferList::create(
         vm, globalObject, reinterpret_cast<Zig::GlobalObject*>(globalObject)->JSBufferListStructure()));
-    putDirect(vm, JSC::Identifier::fromString(vm, "pipes"_s), JSC::constructEmptyArray(globalObject, nullptr, 0));
+    m_pipes.set(vm, this, JSC::constructEmptyArray(globalObject, nullptr, 0));
 
-    if (options == nullptr) {
-        m_emitClose = false;
-        m_autoDestroy = false;
-    } else {
+    if (options != nullptr) {
         JSC::JSValue emitCloseVal = options->getDirect(vm, JSC::Identifier::fromString(vm, "emitClose"_s));
-        m_emitClose = !emitCloseVal.isBoolean() || emitCloseVal.toBoolean(globalObject);
+        if (!emitCloseVal.isBoolean() || emitCloseVal.toBoolean(globalObject))
+            setBool(JSReadableState::Mask::emitClose, true);
         // Has it been destroyed.
         JSC::JSValue autoDestroyVal = options->getDirect(vm, JSC::Identifier::fromString(vm, "autoDestroy"_s));
-        m_autoDestroy = !autoDestroyVal.isBoolean() || autoDestroyVal.toBoolean(globalObject);
+        if (!autoDestroyVal.isBoolean() || autoDestroyVal.toBoolean(globalObject))
+            setBool(JSReadableState::Mask::autoDestroy, true);
     }
 
     // Indicates whether the stream has finished destroying.
-    putDirect(vm, JSC::Identifier::fromString(vm, "errored"_s), JSC::jsNull());
+    m_errored.set(vm, this, JSC::jsNull());
 
     // Ref the piped dest which we need a drain event on it
     // type: null | Writable | Set<Writable>.
-    auto defaultEncodingIdent = JSC::Identifier::fromString(vm, "defaultEncoding"_s);
     if (options == nullptr) {
-        putDirect(vm, WTFMove(defaultEncodingIdent), JSC::jsString(vm, WTF::String("utf8"_s)));
+        m_defaultEncoding.set(vm, this, JSC::jsString(vm, WTF::String("utf8"_s)));
     } else {
-        JSC::JSValue defaultEncodingVal = getDirect(vm, defaultEncodingIdent);
+        JSC::JSValue defaultEncodingVal = getDirect(vm, JSC::Identifier::fromString(vm, "defaultEncoding"_s));
         if (defaultEncodingVal) {
-            putDirect(vm, WTFMove(defaultEncodingIdent), defaultEncodingVal);
+            m_defaultEncoding.set(vm, this, defaultEncodingVal);
         } else {
-            putDirect(vm, WTFMove(defaultEncodingIdent), JSC::jsString(vm, WTF::String("utf8"_s)));
+            m_defaultEncoding.set(vm, this, JSC::jsString(vm, WTF::String("utf8"_s)));
         }
     }
 
-    putDirect(vm, JSC::Identifier::fromString(vm, "awaitDrainWriters"_s), JSC::jsNull());
+    m_awaitDrainWriters.set(vm, this, JSC::jsNull());
 
-    auto decoderIdent = JSC::Identifier::fromString(vm, "decoder"_s);
-    auto encodingIdent = JSC::Identifier::fromString(vm, "encoding"_s);
     if (options == nullptr) {
-        putDirect(vm, WTFMove(decoderIdent), JSC::jsNull());
-        putDirect(vm, WTFMove(encodingIdent), JSC::jsNull());
+        m_decoder.set(vm, this, JSC::jsNull());
+        m_encoding.set(vm, this, JSC::jsNull());
     } else {
-        JSC::JSValue encodingVal = options->getDirect(vm, encodingIdent);
+        JSC::JSValue encodingVal = options->getDirect(vm, JSC::Identifier::fromString(vm, "encoding"_s));
         if (encodingVal) {
             auto constructor = reinterpret_cast<Zig::GlobalObject*>(globalObject)->JSStringDecoder();
             auto constructData = JSC::getConstructData(constructor);
             MarkedArgumentBuffer args;
             args.append(encodingVal);
             JSObject* decoder = JSC::construct(globalObject, constructor, constructData, args);
-            putDirect(vm, WTFMove(decoderIdent), decoder);
-            putDirect(vm, WTFMove(encodingIdent), encodingVal);
+            m_decoder.set(vm, this, decoder);
+            m_encoding.set(vm, this, encodingVal);
         } else {
-            putDirect(vm, WTFMove(decoderIdent), JSC::jsNull());
-            putDirect(vm, WTFMove(encodingIdent), JSC::jsNull());
+            m_decoder.set(vm, this, JSC::jsNull());
+            m_encoding.set(vm, this, JSC::jsNull());
         }
     }
 }
@@ -125,21 +118,33 @@ JSC::GCClient::IsoSubspace* JSReadableState::subspaceForImpl(JSC::VM& vm)
         [](auto& spaces, auto&& space) { spaces.m_subspaceForReadableState = WTFMove(space); });
 }
 
+template<typename Visitor>
+void JSReadableState::visitChildrenImpl(JSCell* cell, Visitor& visitor)
+{
+    JSReadableState* state = jsCast<JSReadableState*>(cell);
+    ASSERT_GC_OBJECT_INHERITS(state, info());
+    Base::visitChildren(state, visitor);
+    visitor.append(state->m_buffer);
+    visitor.append(state->m_pipes);
+    visitor.append(state->m_errored);
+    visitor.append(state->m_defaultEncoding);
+    visitor.append(state->m_awaitDrainWriters);
+    visitor.append(state->m_decoder);
+    visitor.append(state->m_encoding);
+}
+DEFINE_VISIT_CHILDREN(JSReadableState);
+
 STATIC_ASSERT_ISO_SUBSPACE_SHARABLE(JSReadableStatePrototype, JSReadableStatePrototype::Base);
 
 JSC_DEFINE_CUSTOM_GETTER(jsReadableState_pipesCount, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, PropertyName attributeName))
 {
     auto& vm = JSC::getVM(lexicalGlobalObject);
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    JSObject* thisObject = JSC::jsDynamicCast<JSObject*>(JSValue::decode(thisValue));
-    if (!thisObject) {
-        RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined()));
-    }
-    JSC::JSValue pipesVal = thisObject->getDirect(vm, JSC::Identifier::fromString(vm, "pipes"_s));
-    if (!pipesVal) {
+    JSReadableState* state = JSC::jsDynamicCast<JSReadableState*>(JSValue::decode(thisValue));
+    if (!state) {
         RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined()));
     }
-    JSArray* pipes = JSC::jsDynamicCast<JSArray*>(pipesVal);
+    JSArray* pipes = JSC::jsDynamicCast<JSArray*>(state->m_pipes.get());
     if (!pipes) {
         RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined()));
     }
@@ -179,7 +184,7 @@ JSReadableState_NULLABLE_BOOLEAN_GETTER_SETTER(flowing)
 
 #undef JSReadableState_NULLABLE_BOOLEAN_GETTER_SETTER
 
-#define JSReadableState_GETTER_SETTER(NAME, TYPE)                                                                                      \
+#define JSReadableState_NUMBER_GETTER_SETTER(NAME) \
     static JSC_DECLARE_CUSTOM_GETTER(jsReadableState_##NAME); \
     JSC_DEFINE_CUSTOM_GETTER(jsReadableState_##NAME, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, PropertyName attributeName)) \
     {                                                                                                                                              \
@@ -189,7 +194,7 @@ JSReadableState_NULLABLE_BOOLEAN_GETTER_SETTER(flowing)
         if (!state) { \
             RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined())); \
         } \
-        RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(JSC::js##TYPE(state->m_##NAME))); \
+        RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(JSC::jsNumber(state->m_##NAME))); \
     } \
     \
     static JSC_DECLARE_CUSTOM_SETTER(setJSReadableState_##NAME); \
@@ -201,17 +206,43 @@ JSReadableState_NULLABLE_BOOLEAN_GETTER_SETTER(flowing)
         if (!state) { \
             RETURN_IF_EXCEPTION(throwScope, false); \
         } \
-        state->m_##NAME = JSC::JSValue::decode(encodedValue).to##TYPE(lexicalGlobalObject); \
+        state->m_##NAME = JSC::JSValue::decode(encodedValue).toNumber(lexicalGlobalObject); \
         RETURN_IF_EXCEPTION(throwScope, false); \
         RELEASE_AND_RETURN(throwScope, true); \
     }
 
-#define JSReadableState_BOOLEAN_GETTER_SETTER(NAME)       \
-    JSReadableState_GETTER_SETTER(NAME, Boolean)
+JSReadableState_NUMBER_GETTER_SETTER(length)
+JSReadableState_NUMBER_GETTER_SETTER(highWaterMark)
+
+#undef JSReadableState_NUMBER_GETTER_SETTER
 
-#define JSReadableState_NUMBER_GETTER_SETTER(NAME)        \
-    JSReadableState_GETTER_SETTER(NAME, Number)
+#define JSReadableState_BOOLEAN_GETTER_SETTER(NAME)       \
+    static JSC_DECLARE_CUSTOM_GETTER(jsReadableState_##NAME); \
+    JSC_DEFINE_CUSTOM_GETTER(jsReadableState_##NAME, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, PropertyName attributeName)) \
+    {                                                                                                                                              \
+        auto& vm = JSC::getVM(lexicalGlobalObject); \
+        auto throwScope = DECLARE_THROW_SCOPE(vm); \
+        JSReadableState* state = JSC::jsDynamicCast<JSReadableState*>(JSValue::decode(thisValue)); \
+        if (!state) { \
+            RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined())); \
+        } \
+        RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(JSC::jsBoolean(state->getBool(JSReadableState::Mask::NAME)))); \
+    } \
+    \
+    static JSC_DECLARE_CUSTOM_SETTER(setJSReadableState_##NAME); \
+    JSC_DEFINE_CUSTOM_SETTER(setJSReadableState_##NAME, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, EncodedJSValue encodedValue, PropertyName attributeName)) \
+    { \
+        auto& vm = JSC::getVM(lexicalGlobalObject); \
+        auto throwScope = DECLARE_THROW_SCOPE(vm); \
+        JSReadableState* state = JSC::jsDynamicCast<JSReadableState*>(JSValue::decode(thisValue)); \
+        if (!state) { \
+            RETURN_IF_EXCEPTION(throwScope, false); \
+        } \
+        state->setBool(JSReadableState::Mask::NAME, JSC::JSValue::decode(encodedValue).toBoolean(lexicalGlobalObject)); \
+        RELEASE_AND_RETURN(throwScope, true); \
+    }
 
+JSReadableState_BOOLEAN_GETTER_SETTER(objectMode)
 JSReadableState_BOOLEAN_GETTER_SETTER(ended)
 JSReadableState_BOOLEAN_GETTER_SETTER(endEmitted)
 JSReadableState_BOOLEAN_GETTER_SETTER(reading)
@@ -231,12 +262,43 @@ JSReadableState_BOOLEAN_GETTER_SETTER(multiAwaitDrain)
 JSReadableState_BOOLEAN_GETTER_SETTER(readingMore)
 JSReadableState_BOOLEAN_GETTER_SETTER(dataEmitted)
 
-JSReadableState_NUMBER_GETTER_SETTER(length)
-JSReadableState_NUMBER_GETTER_SETTER(highWaterMark)
-
-#undef JSReadableState_NUMBER_GETTER_SETTER
 #undef JSReadableState_BOOLEAN_GETTER_SETTER
-#undef JSReadableState_GETTER_SETTER
+
+#define JSReadableState_JSVALUE_GETTER_SETTER(NAME) \
+    static JSC_DECLARE_CUSTOM_GETTER(jsReadableState_##NAME); \
+    JSC_DEFINE_CUSTOM_GETTER(jsReadableState_##NAME, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, PropertyName attributeName)) \
+    { \
+        auto& vm = JSC::getVM(lexicalGlobalObject); \
+        auto throwScope = DECLARE_THROW_SCOPE(vm); \
+        JSReadableState* state = JSC::jsDynamicCast<JSReadableState*>(JSValue::decode(thisValue)); \
+        if (!state) { \
+            RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined())); \
+        } \
+        RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(state->m_##NAME.get())); \
+    } \
+    static JSC_DECLARE_CUSTOM_SETTER(setJSReadableState_##NAME); \
+    JSC_DEFINE_CUSTOM_SETTER(setJSReadableState_##NAME, (JSGlobalObject * lexicalGlobalObject, EncodedJSValue thisValue, EncodedJSValue encodedValue, PropertyName attributeName)) \
+    { \
+        auto& vm = JSC::getVM(lexicalGlobalObject); \
+        auto throwScope = DECLARE_THROW_SCOPE(vm); \
+        JSReadableState* state = JSC::jsDynamicCast<JSReadableState*>(JSValue::decode(thisValue)); \
+        if (!state) { \
+            RETURN_IF_EXCEPTION(throwScope, false); \
+        } \
+        auto value = JSC::JSValue::decode(encodedValue); \
+        state->m_##NAME.set(vm, state, value); \
+        RELEASE_AND_RETURN(throwScope, true); \
+    }
+
+JSReadableState_JSVALUE_GETTER_SETTER(buffer)
+JSReadableState_JSVALUE_GETTER_SETTER(pipes)
+JSReadableState_JSVALUE_GETTER_SETTER(errored)
+JSReadableState_JSVALUE_GETTER_SETTER(defaultEncoding)
+JSReadableState_JSVALUE_GETTER_SETTER(awaitDrainWriters)
+JSReadableState_JSVALUE_GETTER_SETTER(decoder)
+JSReadableState_JSVALUE_GETTER_SETTER(encoding)
+
+#undef JSReadableState_JSVALUE_GETTER_SETTER
 
 #define JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(NAME) \
     { #NAME ""_s, static_cast<unsigned>(JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { HashTableValue::GetterSetterType, jsReadableState_##NAME, setJSReadableState_##NAME } }
@@ -248,6 +310,7 @@ static const HashTableValue JSReadableStatePrototypeTableValues[]
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(paused),
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(flowing),
           
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(objectMode),
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(ended),
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(endEmitted),
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(reading),
@@ -269,6 +332,14 @@ static const HashTableValue JSReadableStatePrototypeTableValues[]
 
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(length),
           JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(highWaterMark),
+
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(buffer),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(pipes),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(errored),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(defaultEncoding),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(awaitDrainWriters),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(decoder),
+          JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE(encoding),
       };
 
 #undef JSReadableState_GETTER_SETTER_HASH_TABLE_VALUE