Ported parts of : Refactor Node Processing

* Node processing works on the concept of process groups.
* A node group can be inherited, run on main thread, or a sub-thread.
* Groups can be ordered.
* Process priority is now present for physics.
This is the first steps towards implementing godotengine/godot-proposals#6424.
No threading or thread guards exist yet in most of the scene code other than Node. That will have to be added later.
- reduz
godotengine/godot@98c655e
- Only got the smaller improvements, and the thread safety for Node and SceneTree. I'm planning to implement a similar system, but I have a different way of doing it in mind.

core/containers/hash_map.h

@@ -99,7 +99,7 @@ class HashMap {
 	}
 
 	void clear() {
-		if (elements == nullptr) {
+		if (elements == nullptr || num_elements == 0) {
 			return;
 		}
 		uint32_t capacity = hash_table_size_primes[capacity_index];
@@ -517,7 +517,7 @@ class HashMap {
 	}
 
 	bool _lookup_pos(const TKey &p_key, uint32_t &r_pos) const {
-		if (elements == nullptr) {
+		if (elements == nullptr || num_elements == 0) {
 			return false; // Failed lookups, no elements
 		}
 

core/containers/hash_set.h

@@ -79,7 +79,7 @@ class HashSet {
 	}
 
 	bool _lookup_pos(const TKey &p_key, uint32_t &r_pos) const {
-		if (keys == nullptr) {
+		if (keys == nullptr || num_elements == 0) {
 			return false; // Failed lookups, no elements
 		}
 
@@ -236,7 +236,7 @@ class HashSet {
 	}
 
 	void clear() {
-		if (keys == nullptr) {
+		if (keys == nullptr || num_elements == 0) {
 			return;
 		}
 		uint32_t capacity = hash_table_size_primes[capacity_index];

core/containers/local_vector.h

@@ -30,11 +30,11 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
 /*************************************************************************/
 
-#include "core/error/error_macros.h"
-#include "core/os/memory.h"
 #include "core/containers/pool_vector.h"
 #include "core/containers/sort_array.h"
 #include "core/containers/vector.h"
+#include "core/error/error_macros.h"
+#include "core/os/memory.h"
 
 template <class T, class U = uint32_t, bool force_trivial = false>
 class LocalVector {
@@ -94,11 +94,13 @@ class LocalVector {
 		}
 	}
 
-	void erase(const T &p_val) {
+	_FORCE_INLINE_ bool erase(const T &p_val) {
 		int64_t idx = find(p_val);
 		if (idx >= 0) {
 			remove(idx);
+			return true;
 		}
+		return false;
 	}
 
 	U erase_multiple_unordered(const T &p_val) {
@@ -280,7 +282,7 @@ class LocalVector {
 			data[i] = r[i];
 		}
 	}
-	
+
 	inline LocalVector &operator=(const LocalVector &p_from) {
 		resize(p_from.size());
 		for (U i = 0; i < p_from.count; i++) {

core/containers/vector.h

@@ -37,9 +37,9 @@
  */
 
 #include "core/containers/cowdata.h"
+#include "core/containers/sort_array.h"
 #include "core/error/error_macros.h"
 #include "core/os/memory.h"
-#include "core/containers/sort_array.h"
 
 template <class T>
 class VectorWriteProxy {
@@ -65,11 +65,13 @@ class Vector {
 	bool push_back(T p_elem);
 
 	void remove(int p_index) { _cowdata.remove(p_index); }
-	void erase(const T &p_val) {
+	_FORCE_INLINE_ bool erase(const T &p_val) {
 		int idx = find(p_val);
 		if (idx >= 0) {
 			remove(idx);
+			return true;
 		}
+		return false;
 	};
 	void invert();
 

core/object/object.cpp

@@ -391,6 +391,10 @@ bool Object::_predelete() {
 	return _predelete_ok;
 }
 
+void Object::cancel_free() {
+	_predelete_ok = 0;
+}
+
 void Object::_postinitialize() {
 	_class_ptr = _get_class_namev();
 	_initialize_classv();
@@ -957,10 +961,6 @@ void Object::property_list_changed_notify() {
 	_change_notify();
 }
 
-void Object::cancel_delete() {
-	_predelete_ok = true;
-}
-
 ObjectRC *Object::_use_rc() {
 	// The RC object is lazily created the first time it's requested;
 	// that way, there's no need to allocate and release it at all if this Object
@@ -1734,6 +1734,7 @@ void Object::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("tr", "message"), &Object::tr);
 
 	ClassDB::bind_method(D_METHOD("is_queued_for_deletion"), &Object::is_queued_for_deletion);
+	ClassDB::bind_method(D_METHOD("cancel_free"), &Object::cancel_free);
 
 	ClassDB::add_virtual_method("Object", MethodInfo("free"), false);
 

core/object/object.h

@@ -33,12 +33,12 @@
 #include "core/containers/hash_map.h"
 #include "core/containers/list.h"
 #include "core/containers/rb_map.h"
+#include "core/containers/rb_set.h"
+#include "core/containers/vmap.h"
 #include "core/object/object_id.h"
 #include "core/os/rw_lock.h"
 #include "core/os/safe_refcount.h"
-#include "core/containers/rb_set.h"
 #include "core/variant/variant.h"
-#include "core/containers/vmap.h"
 
 #include <atomic>
 
@@ -565,8 +565,6 @@ class Object {
 	static void get_valid_parents_static(List<String> *p_parents);
 	static void _get_valid_parents_static(List<String> *p_parents);
 
-	void cancel_delete();
-
 	void property_list_changed_notify();
 	virtual void _changed_callback(Object *p_changed, const char *p_prop);
 
@@ -809,6 +807,8 @@ class Object {
 
 	void clear_internal_resource_paths();
 
+	void cancel_free();
+
 	Object();
 	virtual ~Object();
 };

core/os/thread.h

@@ -96,6 +96,8 @@ class Thread {
 	// get the ID of the main thread
 	_FORCE_INLINE_ static ID get_main_id() { return main_thread_id; }
 
+	_FORCE_INLINE_ static bool is_main_thread() { return get_caller_id() == main_thread_id; }
+
 	static Error set_name(const String &p_name);
 
 	void start(Thread::Callback p_callback, void *p_user, const Settings &p_settings = Settings());
@@ -110,6 +112,8 @@ class Thread {
 	_FORCE_INLINE_ static ID get_caller_id() { return 0; }
 	// get the ID of the main thread
 	_FORCE_INLINE_ static ID get_main_id() { return 0; }
+
+	_FORCE_INLINE_ static bool is_main_thread() { return true; }
 
 	static Error set_name(const String &p_name) { return ERR_UNAVAILABLE; }
 

doc/classes/Object.xml

@@ -100,6 +100,12 @@
 				Returns [code]true[/code] if the object can translate strings. See [method set_message_translation] and [method tr].
 			</description>
 		</method>
+		<method name="cancel_free">
+			<return type="void" />
+			<description>
+				If this method is called during [constant NOTIFICATION_PREDELETE], this object will reject being freed and will remain allocated. This is mostly an internal function used for error handling to avoid the user from freeing objects when they are not intended to.
+			</description>
+		</method>
 		<method name="connect">
 			<return type="int" enum="Error" />
 			<argument index="0" name="signal" type="StringName" />

scene/main/node.cpp

@@ -179,6 +179,12 @@ void Node::_notification(int p_notification) {
 			data.in_constructor = false;
 		} break;
 		case NOTIFICATION_PREDELETE: {
+			if (data.inside_tree && !Thread::is_main_thread()) {
+				cancel_free();
+				ERR_PRINT("Attempted to free a node that is currently added to the SceneTree from a thread. This is not permitted, use queue_free() instead. Node has not been freed.");
+				return;
+			}
+
 			if (data.parent) {
 				data.parent->remove_child(this);
 			}
@@ -420,6 +426,7 @@ void Node::_propagate_exit_tree() {
 }
 
 void Node::move_child(Node *p_child, int p_pos) {
+	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Moving child node positions inside the SceneTree is only allowed from the main thread. Use call_deferred(\"move_child\",child,index).");
 	ERR_FAIL_NULL(p_child);
 	ERR_FAIL_INDEX_MSG(static_cast<uint32_t>(p_pos), data.children.size() + 1, vformat("Invalid new child position: %d.", p_pos));
 	ERR_FAIL_COND_MSG(p_child->data.parent != this, "Child is not a child of this node.");
@@ -1270,6 +1277,8 @@ void Node::_set_name_nocheck(const StringName &p_name) {
 }
 
 void Node::set_name(const String &p_name) {
+	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Changing the name to nodes inside the SceneTree is only allowed from the main thread. Use call_deferred(\"set_name\",new_name).");
+
 	String name = p_name.validate_node_name();
 
 	ERR_FAIL_COND(name == "");
@@ -1504,6 +1513,7 @@ void Node::_add_child_nocheck(Node *p_child, const StringName &p_name) {
 }
 
 void Node::add_child(Node *p_child, bool p_force_readable_name) {
+	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Adding children to a node inside the SceneTree is only allowed from the main thread. Use call_deferred(\"add_child\",node).");
 	ERR_FAIL_NULL(p_child);
 	ERR_FAIL_COND_MSG(p_child == this, vformat("Can't add child '%s' to itself.", p_child->get_name())); // adding to itself!
 	ERR_FAIL_COND_MSG(p_child->data.parent, vformat("Can't add child '%s' to '%s', already has a parent '%s'.", p_child->get_name(), get_name(), p_child->data.parent->get_name())); //Fail if node has a parent
@@ -1532,6 +1542,7 @@ void Node::add_child_below_node(Node *p_node, Node *p_child, bool p_force_readab
 }
 
 void Node::remove_child(Node *p_child) {
+	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Removing children from a node inside the SceneTree is only allowed from the main thread. Use call_deferred(\"remove_child\",node).");
 	ERR_FAIL_NULL(p_child);
 	ERR_FAIL_COND_MSG(data.blocked > 0, "Parent node is busy setting up children, remove_node() failed. Consider using call_deferred(\"remove_child\", child) instead.");
 
@@ -1881,6 +1892,8 @@ void Node::_acquire_unique_name_in_owner() {
 }
 
 void Node::set_unique_name_in_owner(bool p_enabled) {
+	ERR_FAIL_COND_MSG(is_inside_tree() && !Thread::is_main_thread(), "This function in this node can only be accessed from the main thread. Use call_deferred() instead.");
+
 	if (data.unique_name_in_owner == p_enabled) {
 		return;
 	}
@@ -1902,6 +1915,8 @@ bool Node::is_unique_name_in_owner() const {
 }
 
 void Node::set_owner(Node *p_owner) {
+	ERR_FAIL_COND_MSG(is_inside_tree() && !Thread::is_main_thread(), "This function in this node can only be accessed from the main thread. Use call_deferred() instead.");
+
 	if (data.owner) {
 		if (data.unique_name_in_owner) {
 			_release_unique_name_in_owner();

scene/main/node.h

@@ -467,6 +467,10 @@ class Node : public Object {
 	void set_process_unhandled_key_input(bool p_enable);
 	bool is_processing_unhandled_key_input() const;
 
+	_FORCE_INLINE_ bool _is_any_processing() const {
+		return data.idle_process || data.idle_process_internal || data.physics_process || data.physics_process_internal;
+	}
+
 	int get_position_in_parent() const;
 
 	Node *duplicate(int p_flags = DUPLICATE_GROUPS | DUPLICATE_SIGNALS | DUPLICATE_SCRIPTS) const;