From 660e4b3c5b4e2decd5100f8890041b1934824fe3 Mon Sep 17 00:00:00 2001
From: Ciro Spaciari MacBook <ciro@anthropic.com>
Date: Tue, 3 Feb 2026 20:13:12 -0800
Subject: [PATCH] fix(yoga): free YGNode in YogaNodeImpl destructor to fix LSAN
 leak

---
 src/bun.js/bindings/YogaNodeImpl.cpp | 39 ++++++++++++++++++++++++----
 src/bun.js/bindings/YogaNodeImpl.h   |  1 +
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/bun.js/bindings/YogaNodeImpl.cpp b/src/bun.js/bindings/YogaNodeImpl.cpp
index 734b5882f5..c20c20dba1 100644
--- a/src/bun.js/bindings/YogaNodeImpl.cpp
+++ b/src/bun.js/bindings/YogaNodeImpl.cpp
@@ -27,8 +27,19 @@ YogaNodeImpl::YogaNodeImpl(YGConfigRef config)
 
 YogaNodeImpl::~YogaNodeImpl()
 {
-    // Don't call YGNodeFree here - let JS finalizer handle it to control timing
-    // This avoids double-free issues during GC when nodes may be freed in arbitrary order
+    // 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;
 }
 
@@ -79,12 +90,30 @@ YogaNodeImpl* YogaNodeImpl::fromYGNode(YGNodeRef nodeRef)
 
 void YogaNodeImpl::replaceYogaNode(YGNodeRef newNode)
 {
-    // Don't access old YGNode - it might be freed already
-    // Let Yoga handle cleanup of the old node
-    m_yogaNode = 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 66c4db0da0..c21e231598 100644
--- a/src/bun.js/bindings/YogaNodeImpl.h
+++ b/src/bun.js/bindings/YogaNodeImpl.h
@@ -37,6 +37,7 @@ private:
     explicit YogaNodeImpl(YGConfigRef config);
 
     YGNodeRef m_yogaNode;
+    bool m_ownsNode { true };
     JSC::Weak<JSYogaNode> m_wrapper;
 };