Repository: godot
Issue: Collision shapes update (2d and 3d) race condition causing infinite loop
Labels: bug,topic:physics

### Tested versions

- Reproducible in: 4.3.stable, 4.4.beta1, v4.4.dev [61accf060]

### System information

Godot v4.3.stable - TUXEDO OS 24.04.1 LTS - Wayland - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4060 Laptop GPU - 13th Gen Intel(R) Core(TM) i9-13900HX (32 Threads)

### Issue description

When running physics in a separate thread and updating a `CollisionShape2D` or a `CollisionShape3D` (eg. changing the transform) in `_physics_process`, the physics server enters an infinite loop and spams the console with:
```
ERROR: Condition "p_elem->_root != this" is true.
   at: remove (./core/templates/self_list.h:80)
```
If the "run physics in separate thread" option is disabled, this error does not occur.

I have mostly tested 3D physics as this is where I first encountered this issue, so most of my findings are about 3D physics, though it seems to be the same thing for the 2D error.

First of all here is where the error occurs in godot-v4.4.dev [61accf060] :
```
#0  SelfList<GodotCollisionObject3D>::List::remove (this=0x555564c05718, p_elem=0x555561bc8640) at ./core/templates/self_list.h:80
#1  0x0000555558fa5650 in GodotPhysicsServer3D::_update_shapes (this=0x555564c05350) at modules/godot_physics_3d/godot_physics_server_3d.cpp:1730
#2  0x0000555558fa0849 in GodotPhysicsServer3D::body_test_motion (this=0x555564c05350, p_body=..., p_parameters=..., r_result=0x7fffffff89e0) at modules/godot_physics_3d/godot_physics_server_3d.cpp:918
#3  0x000055555cce8818 in PhysicsServer3DWrapMT::body_test_motion (this=0x55556382ea20, p_body=..., p_parameters=..., r_result=0x7fffffff89e0) at servers/physics_server_3d_wrap_mt.h:264
#4  0x000055555b859c3d in PhysicsBody3D::move_and_collide (this=0x555565e64ec0, p_parameters=..., r_result=..., p_test_only=false, p_cancel_sliding=true) at scene/3d/physics/physics_body_3d.cpp:109
#5  0x000055555b80c80b in CharacterBody3D::_move_and_slide_grounded (this=0x555565e64ec0, p_delta=0.016666666666666666, p_was_on_floor=false) at scene/3d/physics/character_body_3d.cpp:164
#6  0x000055555b80c2d1 in CharacterBody3D::move_and_slide (this=0x555565e64ec0) at scene/3d/physics/character_body_3d.cpp:112
#7  0x00005555587a28be in call_with_validated_variant_args_ret_helper<__UnexistingClass, bool>(__UnexistingClass*, bool (__UnexistingClass::*)(), Variant const**, Variant*, IndexSequence<>) (p_instance=0x555565e64ec0, p_method=(bool (__UnexistingClass::*)(__UnexistingClass * const)) 0x55555b80b9d4 <CharacterBody3D::move_and_slide()>, p_args=0x7fffffffa1c8, r_ret=0x7fffffffa1b0) at ./core/variant/binder_common.h:375
#8  0x000055555879dc60 in call_with_validated_object_instance_args_ret<__UnexistingClass, bool> (base=0x555565e64ec0, p_method=(bool (__UnexistingClass::*)(__UnexistingClass * const)) 0x55555b80b9d4 <CharacterBody3D::move_and_slide()>, p_args=0x7fffffffa1c8, r_ret=0x7fffffffa1b0) at ./core/variant/binder_common.h:662
#9  0x0000555558794947 in MethodBindTR<bool>::validated_call (this=0x555565561b00, p_object=0x555565e64ec0, p_args=0x7fffffffa1c8, r_ret=0x7fffffffa1b0) at ./core/object/method_bind.h:536
#10 0x0000555558ad1340 in GDScriptFunction::call (this=0x555565e96fe0, p_instance=0x555565cda400, p_args=0x7fffffffc858, p_argcount=1, r_err=..., p_state=0x0) at modules/gdscript/gdscript_vm.cpp:2250
#11 0x000055555894ec29 in GDScriptInstance::callp (this=0x555565cda400, p_method=..., p_args=0x7fffffffc858, p_argcount=1, r_error=...) at modules/gdscript/gdscript.cpp:2073
#12 0x000055555af899fa in Node::_gdvirtual__physics_process_call (this=0x555565e64ec0, arg1=0.016666666666666666) at scene/main/node.h:381
#13 0x000055555af68af1 in Node::_notification (this=0x555565e64ec0, p_notification=16) at scene/main/node.cpp:59
[...]
```

`GodotPhysicsServer3D::_update_shapes` tries to remove the first element of `GodotPhysicsServer3D::pending_shape_update_list`, but it can't because `pending_shape_update_list.first()` is already out of the list. Since this is executed until the list is empty, the thread is caught in an infinite loop.
Checking the data stored in this first element, I found that `_root`, `_next` and `_prev` are all `nullptr` (this was true every time I checked), meaning that it was "somewhat cleanely" removed from the list. From what I've gathered, this situation should not be possible unless `remove` and `add` operations are done at the same time from separate threads.

`pending_shape_update_list` is referenced in `GodotPhysicsServer3D::_update_shapes` and in 5 other function in `GodotCollisionObject3D`:
- `GodotCollisionObject3D::add_shape`
- `GodotCollisionObject3D::set_shape`
- `GodotCollisionObject3D::set_shape_transform`
- `GodotCollisionObject3D::set_shape_disabled`
- `GodotCollisionObject3D::remove_shape`

Those 5 functions try to `add` an element to the list to queue it's update.

From my understainding of the threading architecture, all of these functions referencing `pending_shape_update_list` are (indirectly) scheduled to be called in some way by `PhysicsServer3DWrapMT`. If they are scheduled to run in parallel when `run_on_separate_thread` is enabled, it would cause this race condition. But I'm not sure if that is the exact reason behind this issue.

Here are some things I find wierd / I don't yet understaind:
- When the collision shapes are updated during the process frame (and not the physics frame) this bug does not seem to be triggered. If this issue were caused by the `PhysicsServer3DWrapMT`'s scheduling, shouldn't it still trigger when pushing updates inside `_process` ?
- I've not had this bug happen when only one physics object is present in the scene, even after 40+min of it running. With two physics objects, it only takes a minute on average.
- I've not seen `ERR_FAIL_COND(p_elem->_root)` fail inside `SelfList<>::List::add`. If the issue were only due to `add` and `remove` being run at the same time, I would expect this condition to be met at least some of the time. This could be because it is rare enough for me not to have seen it though.

### Steps to reproduce

- Enable the advanced `run_on_separate_thread` option
- Create some physics objects
- Change the physics objects' collision shapes (eg. add/remove collision shapes, change the transform) during the physics frame
- Wait for godot to freeze and start screaming in pain

### Minimal reproduction project (MRP)

MRP: [mrp.zip](https://github.com/user-attachments/files/18497651/mrp.zip)