From fcee6de2a9196bb242bc53e71b5f1c3fc2ae1105 Mon Sep 17 00:00:00 2001
From: Ciro Spaciari MacBook <ciro@anthropic.com>
Date: Tue, 3 Feb 2026 21:33:24 -0800
Subject: [PATCH] revert(yoga): remove YGNodeFinalize from destructor to fix GC
 sweep crash

Calling delete on yoga::Node during GC finalization is unsafe because
the GC sweeps objects in arbitrary order and the allocator behavior
on musl can cause null-pointer dereferences. The LSAN suppressions
in test/leaksan.supp handle the leak detection false positives.

This reverts the destructor and replaceYogaNode changes from 660e4b3c
while keeping the LSAN suppressions from c17c345d.
---
 src/bun.js/bindings/YogaNodeImpl.cpp | 36 ----------------------------
 src/bun.js/bindings/YogaNodeImpl.h   |  1 -
 2 files changed, 37 deletions(-)

diff --git a/src/bun.js/bindings/YogaNodeImpl.cpp b/src/bun.js/bindings/YogaNodeImpl.cpp
index c20c20dba1..b1a00425f1 100644
--- a/src/bun.js/bindings/YogaNodeImpl.cpp
+++ b/src/bun.js/bindings/YogaNodeImpl.cpp
@@ -27,19 +27,6 @@ YogaNodeImpl::YogaNodeImpl(YGConfigRef config)
 
 YogaNodeImpl::~YogaNodeImpl()
 {
-    // Free the underlying Yoga node if it hasn't been freed already.
-    // When the user calls .free() explicitly, replaceYogaNode(nullptr) sets
-    // m_yogaNode to null first, so this guard prevents double-free.
-    // When another YogaNodeImpl takes over via replaceYogaNode(), m_ownsNode
-    // is set to false so we skip the free and avoid double-free.
-    if (m_yogaNode && m_ownsNode) {
-        // Use YGNodeFinalize instead of YGNodeFree: it frees the node's
-        // memory without disconnecting it from its owner or children.
-        // This is safe during GC, where nodes in the same tree may be
-        // swept in arbitrary order and parent/child pointers may already
-        // be dangling.
-        YGNodeFinalize(m_yogaNode);
-    }
     m_yogaNode = nullptr;
 }
 
@@ -90,30 +77,7 @@ YogaNodeImpl* YogaNodeImpl::fromYGNode(YGNodeRef nodeRef)
 
 void YogaNodeImpl::replaceYogaNode(YGNodeRef newNode)
 {
-    if (newNode) {
-        // Free the old node if we are replacing it with a different one.
-        // This prevents leaks when, e.g., the clone path creates a throwaway
-        // YGNode via create(nullptr) and immediately replaces it.
-        if (m_yogaNode && m_yogaNode != newNode && m_ownsNode) {
-            YGNodeFinalize(m_yogaNode);
-        }
-
-        // If another YogaNodeImpl currently owns this YGNode (e.g. clone
-        // path where YGNodeClone shares children), mark the previous owner
-        // as non-owning so it will not try to free the node in its destructor.
-        // We do NOT clear its m_yogaNode pointer because the original node
-        // still needs it for operations like getWidth(), calculateLayout(), etc.
-        auto* previousOwner = static_cast<YogaNodeImpl*>(YGNodeGetContext(newNode));
-        if (previousOwner && previousOwner != this) {
-            previousOwner->m_ownsNode = false;
-        }
-        YGNodeSetContext(newNode, this);
-    }
-
-    // When newNode is null (called from .free() after YGNodeFree), the old
-    // m_yogaNode was already freed by the caller -- just clear the pointer.
     m_yogaNode = newNode;
-    m_ownsNode = (newNode != nullptr);
 }
 
 } // namespace Bun
diff --git a/src/bun.js/bindings/YogaNodeImpl.h b/src/bun.js/bindings/YogaNodeImpl.h
index c21e231598..66c4db0da0 100644
--- a/src/bun.js/bindings/YogaNodeImpl.h
+++ b/src/bun.js/bindings/YogaNodeImpl.h
@@ -37,7 +37,6 @@ private:
     explicit YogaNodeImpl(YGConfigRef config);
 
     YGNodeRef m_yogaNode;
-    bool m_ownsNode { true };
     JSC::Weak<JSYogaNode> m_wrapper;
 };