godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Error when removing a phycics node during a physics callback 

* This behavior is not allowed, the error text suggests using call_deferred().
* Added a check in Node::remove_child to prevent future crashes of this type.
* Fixed a performance regression introduced by #36244.

Fixes #63718, probably other crashes too.
This commit is contained in:
Juan Linietsky 2023-01-07 12:12:24 +01:00
parent 163f6f5fe8
commit 398e73c689
9 changed files with 66 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
scene/2d/area_2d.cpp
View File

@ -175,6 +175,7 @@ void Area2D::_body_inout(int p_status, const RID &p_body, ObjectID p_instance, i
return; //does not exist because it was likely removed from the tree
}
lock_callback();
locked = true;
if (body_in) {
@ -224,6 +225,7 @@ void Area2D::_body_inout(int p_status, const RID &p_body, ObjectID p_instance, i
}
locked = false;
unlock_callback();
}
void Area2D::_area_enter_tree(ObjectID p_id) {
@ -268,6 +270,8 @@ void Area2D::_area_inout(int p_status, const RID &p_area, ObjectID p_instance, i
if (!area_in && !E) {
return; //likely removed from the tree
}
lock_callback();
locked = true;
if (area_in) {
@ -317,6 +321,7 @@ void Area2D::_area_inout(int p_status, const RID &p_area, ObjectID p_instance, i
}
locked = false;
unlock_callback();
}
void Area2D::_clear_monitoring() {

20
scene/2d/collision_object_2d.cpp
View File

@ -94,10 +94,14 @@ void CollisionObject2D::_notification(int p_what) {
bool disabled = !is_enabled();
if (!disabled || (disable_mode != DISABLE_MODE_REMOVE)) {
if (area) {
PhysicsServer2D::get_singleton()->area_set_space(rid, RID());
if (callback_lock > 0) {
ERR_PRINT("Removing a CollisionObject node during a physics callback is not allowed and will cause undesired behavior. Remove with call_deferred() instead.");
} else {
PhysicsServer2D::get_singleton()->body_set_space(rid, RID());
if (area) {
PhysicsServer2D::get_singleton()->area_set_space(rid, RID());
} else {
PhysicsServer2D::get_singleton()->body_set_space(rid, RID());
}
}
}
@ -225,10 +229,14 @@ void CollisionObject2D::_apply_disabled() {
switch (disable_mode) {
case DISABLE_MODE_REMOVE: {
if (is_inside_tree()) {
if (area) {
PhysicsServer2D::get_singleton()->area_set_space(rid, RID());
if (callback_lock > 0) {
ERR_PRINT("Disabling a CollisionObject node during a physics callback is not allowed and will cause undesired behavior. Disable with call_deferred() instead.");
} else {
PhysicsServer2D::get_singleton()->body_set_space(rid, RID());
if (area) {
PhysicsServer2D::get_singleton()->area_set_space(rid, RID());
} else {
PhysicsServer2D::get_singleton()->body_set_space(rid, RID());
}
}
}
} break;

7
scene/2d/collision_object_2d.h
View File

@ -53,6 +53,7 @@ private:
bool area = false;
RID rid;
uint32_t callback_lock = 0;
bool pickable = false;
DisableMode disable_mode = DISABLE_MODE_REMOVE;
@ -83,6 +84,12 @@ private:
void _apply_enabled();
protected:
_FORCE_INLINE_ void lock_callback() { callback_lock++; }
_FORCE_INLINE_ void unlock_callback() {
ERR_FAIL_COND(callback_lock == 0);
callback_lock--;
}
CollisionObject2D(RID p_rid, bool p_area);
void _notification(int p_what);

4
scene/2d/physics_body_2d.cpp
View File

@ -434,6 +434,8 @@ struct _RigidBody2DInOut {
};
void RigidBody2D::_body_state_changed(PhysicsDirectBodyState2D *p_state) {
lock_callback();
set_block_transform_notify(true); // don't want notify (would feedback loop)
if (!freeze || freeze_mode != FREEZE_MODE_KINEMATIC) {
set_global_transform(p_state->get_transform());
@ -527,6 +529,8 @@ void RigidBody2D::_body_state_changed(PhysicsDirectBodyState2D *p_state) {
contact_monitor->locked = false;
}
unlock_callback();
}
void RigidBody2D::_apply_body_mode() {

4
scene/3d/area_3d.cpp
View File

@ -230,6 +230,7 @@ void Area3D::_body_inout(int p_status, const RID &p_body, ObjectID p_instance, i
return; //likely removed from the tree
}
lock_callback();
locked = true;
if (body_in) {
@ -279,6 +280,7 @@ void Area3D::_body_inout(int p_status, const RID &p_body, ObjectID p_instance, i
}
locked = false;
unlock_callback();
}
void Area3D::_clear_monitoring() {
@ -417,6 +419,7 @@ void Area3D::_area_inout(int p_status, const RID &p_area, ObjectID p_instance, i
return; //likely removed from the tree
}
lock_callback();
locked = true;
if (area_in) {
@ -466,6 +469,7 @@ void Area3D::_area_inout(int p_status, const RID &p_area, ObjectID p_instance, i
}
locked = false;
unlock_callback();
}
bool Area3D::is_monitoring() const {

20
scene/3d/collision_object_3d.cpp
View File

@ -100,10 +100,14 @@ void CollisionObject3D::_notification(int p_what) {
bool disabled = !is_enabled();
if (!disabled || (disable_mode != DISABLE_MODE_REMOVE)) {
if (area) {
PhysicsServer3D::get_singleton()->area_set_space(rid, RID());
if (callback_lock > 0) {
ERR_PRINT("Removing a CollisionObject node during a physics callback is not allowed and will cause undesired behavior. Remove with call_deferred() instead.");
} else {
PhysicsServer3D::get_singleton()->body_set_space(rid, RID());
if (area) {
PhysicsServer3D::get_singleton()->area_set_space(rid, RID());
} else {
PhysicsServer3D::get_singleton()->body_set_space(rid, RID());
}
}
}
@ -223,10 +227,14 @@ void CollisionObject3D::_apply_disabled() {
switch (disable_mode) {
case DISABLE_MODE_REMOVE: {
if (is_inside_tree()) {
if (area) {
PhysicsServer3D::get_singleton()->area_set_space(rid, RID());
if (callback_lock > 0) {
ERR_PRINT("Disabling a CollisionObject node during a physics callback is not allowed and will cause undesired behavior. Disable with call_deferred() instead.");
} else {
PhysicsServer3D::get_singleton()->body_set_space(rid, RID());
if (area) {
PhysicsServer3D::get_singleton()->area_set_space(rid, RID());
} else {
PhysicsServer3D::get_singleton()->body_set_space(rid, RID());
}
}
}
} break;

7
scene/3d/collision_object_3d.h
View File

@ -52,6 +52,7 @@ private:
bool area = false;
RID rid;
uint32_t callback_lock = 0;
DisableMode disable_mode = DISABLE_MODE_REMOVE;
@ -97,6 +98,12 @@ private:
protected:
CollisionObject3D(RID p_rid, bool p_area);
_FORCE_INLINE_ void lock_callback() { callback_lock++; }
_FORCE_INLINE_ void unlock_callback() {
ERR_FAIL_COND(callback_lock == 0);
callback_lock--;
}
void _notification(int p_what);
static void _bind_methods();

4
scene/3d/physics_body_3d.cpp
View File

@ -484,6 +484,8 @@ struct _RigidBodyInOut {
};
void RigidBody3D::_body_state_changed(PhysicsDirectBodyState3D *p_state) {
lock_callback();
set_ignore_transform_notification(true);
set_global_transform(p_state->get_transform());
@ -578,6 +580,8 @@ void RigidBody3D::_body_state_changed(PhysicsDirectBodyState3D *p_state) {
contact_monitor->locked = false;
}
unlock_callback();
}
void RigidBody3D::_notification(int p_what) {

10
scene/main/node.cpp
View File

@ -278,7 +278,10 @@ void Node::_propagate_exit_tree() {
//block while removing children
#ifdef DEBUG_ENABLED
SceneDebugger::remove_from_cache(data.scene_file_path, this);
if (!data.scene_file_path.is_empty()) {
// Only remove if file path is set (optimization).
SceneDebugger::remove_from_cache(data.scene_file_path, this);
}
#endif
data.blocked++;
@ -304,7 +307,6 @@ void Node::_propagate_exit_tree() {
}
// exit groups
for (KeyValue<StringName, GroupData> &E : data.grouped) {
data.tree->remove_from_group(E.key, this);
E.value.group = nullptr;
@ -1165,7 +1167,7 @@ void Node::add_sibling(Node *p_sibling, bool p_force_readable_name) {
void Node::remove_child(Node *p_child) {
ERR_FAIL_NULL(p_child);
ERR_FAIL_COND_MSG(data.blocked > 0, "Parent node is busy setting up children, `remove_child()` failed. Consider using `remove_child.call_deferred(child)` instead.");
ERR_FAIL_COND_MSG(data.blocked > 0, "Parent node is busy adding/removing children, `remove_child()` can't be called at this time. Consider using `remove_child.call_deferred(child)` instead.");
int child_count = data.children.size();
Node **children = data.children.ptrw();
@ -1196,11 +1198,13 @@ void Node::remove_child(Node *p_child) {
data.internal_children_back--;
}
data.blocked++;
p_child->_set_tree(nullptr);
//}
remove_child_notify(p_child);
p_child->notification(NOTIFICATION_UNPARENTED);
data.blocked--;
data.children.remove_at(idx);