From 1eb250e17d3cd1c6727519e4aebff479374a420c Mon Sep 17 00:00:00 2001
From: Rindbee <idleman@yeah.net>
Date: Mon, 27 Feb 2023 16:16:51 +0800
Subject: [PATCH] Avoid crash when adjusting a node tree that is not in the
 tree

When node tree `A` is not in the tree, `remove_child(B)` will not
automatically clean up the owners of `B` and `B`'s child nodes.
This is convenient for implementing operations like `replace_by()`,
but may have hidden dangers when manipulating the rest of the tree
`A`.

This commit makes it safe to manipulate the rest of `A` after freeing
`B`.
---
 scene/main/node.cpp | 30 +++++++++++++++++++-----------
 scene/main/node.h   |  2 ++
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/scene/main/node.cpp b/scene/main/node.cpp
index 3aaafeae308..11f2a6af665 100644
--- a/scene/main/node.cpp
+++ b/scene/main/node.cpp
@@ -202,6 +202,10 @@ void Node::_notification(int p_notification) {
 				return;
 			}
 
+			if (data.owner) {
+				_clean_up_owner();
+			}
+
 			if (data.parent) {
 				data.parent->remove_child(this);
 			}
@@ -301,11 +305,7 @@ void Node::_propagate_after_exit_tree() {
 		}
 
 		if (!found) {
-			if (data.unique_name_in_owner) {
-				_release_unique_name_in_owner();
-			}
-			data.owner->data.owned.erase(data.OW);
-			data.owner = nullptr;
+			_clean_up_owner();
 		}
 	}
 
@@ -1876,12 +1876,7 @@ bool Node::is_unique_name_in_owner() const {
 void Node::set_owner(Node *p_owner) {
 	ERR_MAIN_THREAD_GUARD
 	if (data.owner) {
-		if (data.unique_name_in_owner) {
-			_release_unique_name_in_owner();
-		}
-		data.owner->data.owned.erase(data.OW);
-		data.OW = nullptr;
-		data.owner = nullptr;
+		_clean_up_owner();
 	}
 
 	ERR_FAIL_COND(p_owner == this);
@@ -1915,6 +1910,17 @@ Node *Node::get_owner() const {
 	return data.owner;
 }
 
+void Node::_clean_up_owner() {
+	ERR_FAIL_NULL(data.owner); // Sanity check.
+
+	if (data.unique_name_in_owner) {
+		_release_unique_name_in_owner();
+	}
+	data.owner->data.owned.erase(data.OW);
+	data.owner = nullptr;
+	data.OW = nullptr;
+}
+
 Node *Node::find_common_parent_with(const Node *p_node) const {
 	if (this == p_node) {
 		return const_cast<Node *>(p_node);
@@ -2747,6 +2753,8 @@ void Node::replace_by(Node *p_node, bool p_keep_groups) {
 		for (int i = 0; i < get_child_count(); i++) {
 			find_owned_by(data.owner, get_child(i), &owned_by_owner);
 		}
+
+		_clean_up_owner();
 	}
 
 	Node *parent = data.parent;
diff --git a/scene/main/node.h b/scene/main/node.h
index 0b242eaf976..ad19a1b694a 100644
--- a/scene/main/node.h
+++ b/scene/main/node.h
@@ -236,6 +236,8 @@ private:
 	void _release_unique_name_in_owner();
 	void _acquire_unique_name_in_owner();
 
+	void _clean_up_owner();
+
 	_FORCE_INLINE_ void _update_children_cache() const {
 		if (unlikely(data.children_cache_dirty)) {
 			_update_children_cache_impl();