adding nullptr check before destroyShadow calls (#15127)

* adding nullptr check before destroyShadow calls

* fixing nit issue

* Change files

* adding chnage type

* adding correct format

* Apply suggestion from @jonthysell

Adding type as prerelease

Co-authored-by: Jon Thysell <thysell@gmail.com>

* Apply suggestion from @jonthysell

apply dependantChangeTyoe to patch

Co-authored-by: Jon Thysell <thysell@gmail.com>

---------

Co-authored-by: Protik Biswas <protikbiswas100@microsoft.com>
Co-authored-by: Jon Thysell <thysell@gmail.com>

Author: 3 people

change/react-native-windows-da49d3f3-193f-463a-9a3b-7f543a4f30a4.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "adding nullptr check before destroyShadow calls",
+  "packageName": "react-native-windows",
+  "email": "protikbiswas100@microsoft.com",
+  "dependentChangeType": "patch"
+}

vnext/Microsoft.ReactNative/Modules/PaperUIManagerModule.cpp

@@ -501,22 +501,37 @@ class UIManagerModule : public std::enable_shared_from_this<UIManagerModule>, pu
   }
 
   void DropView(int64_t tag, bool removeChildren = true, bool zombieView = false) {
+    // Prevent re-entry and double deletion which can cause memory corruption
+    if (m_nodesBeingDropped.find(tag) != m_nodesBeingDropped.end()) {
+      return;
+    }
+
     if (auto node = m_nodeRegistry.findNode(tag)) {
+      // Mark this node as being dropped
+      m_nodesBeingDropped.insert(tag);
+
       node->onDropViewInstance();
 
       m_nativeUIManager->RemoveView(*node, removeChildren);
 
       if (zombieView)
         node->m_zombie = true;
 
-      for (auto childTag : node->m_children)
+      // Make a copy of children to avoid iterator invalidation
+      auto children = node->m_children;
+      for (auto childTag : children) {
         DropView(childTag, removeChildren, zombieView);
+      }
 
       if (removeChildren)
         node->removeAllChildren();
 
-      if (!zombieView)
+      if (!zombieView) {
         m_nodeRegistry.removeNode(tag);
+      }
+
+      // Remove from tracking set
+      m_nodesBeingDropped.erase(tag);
     }
   }
 
@@ -525,6 +540,8 @@ class UIManagerModule : public std::enable_shared_from_this<UIManagerModule>, pu
   ShadowNodeRegistry m_nodeRegistry;
   std::shared_ptr<NativeUIManager> m_nativeUIManager;
   const React::JSValueObject accessibilityEventTypes = React::JSValueObject{{"typeViewFocused", 8}};
+  // Track nodes being dropped to prevent double-deletion and corruption
+  std::unordered_set<int64_t> m_nodesBeingDropped;
 };
 
 UIManager::UIManager() : m_module(std::make_shared<UIManagerModule>()) {}

vnext/Microsoft.ReactNative/Views/ShadowNodeBase.cpp

@@ -28,11 +28,19 @@ ViewManagerBase *ShadowNodeBase::GetViewManager() const {
 }
 
 void ShadowNodeBase::updateProperties(winrt::Microsoft::ReactNative::JSValueObject &props) {
-  GetViewManager()->UpdateProperties(this, props);
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->UpdateProperties(this, props);
+  }
 }
 
 void ShadowNodeBase::createView(const winrt::Microsoft::ReactNative::JSValueObject &props) {
-  m_view = GetViewManager()->CreateView(this->m_tag, props);
+  auto viewManager = GetViewManager();
+  if (!viewManager) {
+    return;
+  }
+
+  m_view = viewManager->CreateView(this->m_tag, props);
 
   if (g_HasActualSizeProperty == TriBit::Undefined) {
     if (auto uielement = m_view.try_as<xaml::UIElement>()) {
@@ -49,23 +57,38 @@ bool ShadowNodeBase::NeedsForceLayout() {
 void ShadowNodeBase::dispatchCommand(
     const std::string &commandId,
     winrt::Microsoft::ReactNative::JSValueArray &&commandArgs) {
-  GetViewManager()->DispatchCommand(GetView(), commandId, std::move(commandArgs));
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->DispatchCommand(GetView(), commandId, std::move(commandArgs));
+  }
 }
 
 void ShadowNodeBase::removeAllChildren() {
-  GetViewManager()->RemoveAllChildren(GetView());
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->RemoveAllChildren(GetView());
+  }
 }
 
 void ShadowNodeBase::AddView(ShadowNode &child, int64_t index) {
-  this->GetViewManager()->AddView(GetView(), static_cast<ShadowNodeBase &>(child).GetView(), index);
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->AddView(GetView(), static_cast<ShadowNodeBase &>(child).GetView(), index);
+  }
 }
 
 void ShadowNodeBase::RemoveChildAt(int64_t indexToRemove) {
-  GetViewManager()->RemoveChildAt(GetView(), indexToRemove);
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->RemoveChildAt(GetView(), indexToRemove);
+  }
 }
 
 void ShadowNodeBase::onDropViewInstance() {
-  GetViewManager()->OnDropViewInstance(m_view);
+  auto viewManager = GetViewManager();
+  if (viewManager) {
+    viewManager->OnDropViewInstance(m_view);
+  }
   m_handledKeyboardEventHandler = nullptr;
 }
 
@@ -81,7 +104,12 @@ void ShadowNodeBase::ReplaceView(XamlView view) {
 }
 
 void ShadowNodeBase::ReplaceChild(const XamlView &oldChildView, const XamlView &newChildView) {
-  GetViewManager()->ReplaceChild(m_view, oldChildView, newChildView);
+  // Add safety checks to prevent crashes during cleanup
+  auto viewManager = GetViewManager();
+  if (!viewManager || !m_view || !oldChildView || !newChildView) {
+    return;
+  }
+  viewManager->ReplaceChild(m_view, oldChildView, newChildView);
 }
 
 void ShadowNodeBase::ReparentView(XamlView view) {

vnext/Microsoft.ReactNative/Views/ShadowNodeRegistry.cpp

@@ -10,8 +10,12 @@
 namespace Microsoft::ReactNative {
 
 void ShadowNodeDeleter::operator()(ShadowNode *node) {
-  if (node->m_viewManager)
-    node->m_viewManager->destroyShadow(node);
+  if (node && node->m_viewManager) {
+    // Clear the view manager pointer to prevent double deletion
+    auto viewManager = node->m_viewManager;
+    node->m_viewManager = nullptr;
+    viewManager->destroyShadow(node);
+  }
 }
 
 void ShadowNodeRegistry::addRootView(std::unique_ptr<ShadowNode, ShadowNodeDeleter> &&root, int64_t rootViewTag) {

vnext/Microsoft.ReactNative/Views/ViewManagerBase.cpp

@@ -187,7 +187,11 @@ ShadowNode *ViewManagerBase::createShadow() const {
 }
 
 void ViewManagerBase::destroyShadow(ShadowNode *node) const {
-  delete node;
+  if (node) {
+    // Ensure we don't double-delete by clearing the view manager pointer
+    node->m_viewManager = nullptr;
+    delete node;
+  }
 }
 
 void ViewManagerBase::GetExportedCustomBubblingEventTypeConstants(
@@ -293,6 +297,10 @@ void ViewManagerBase::RemoveAllChildren(const XamlView &parent) {}
 
 void ViewManagerBase::ReplaceChild(const XamlView &parent, const XamlView &oldChild, const XamlView &newChild) {
   // ASSERT: Child must either implement or not allow children.
+  // Add null checks to prevent crashes during cleanup
+  if (!parent || !oldChild || !newChild) {
+    return;
+  }
   assert(false);
 }