Segmentation fault on RequestRemoveEntity() from update() or postUpdate()

[jrutgeer]

Description:

Calling SdfEntityCreator::RequestRemoveEntity() on a model entity from update() or postUpdate() causes a segmentation fault, so it seems that RequestRemoveEntity() is intended to be called only from preUpdate().

However the RequestRemoveEntity() code documentation states:

Request an entity deletion. This will insert the request into a queue.

In my interpretation, this implies the intention that RequestRemoveEntity() can be called at any time, and the entities in the queue will be removed when appropriate.

So:

• Either the documentation is correct and this is a bug,
• Or the documentation is not clear that RequestRemoveEntity() can only be called from preUpdate().

More info:

Here is an example repository. It is a system that:

• Looks for models named box_preupdate / box_update / box_postupdate,
• And, if these exist, calls RequestRemoveEntity() on the corresponding model entity, respectively from preUpdate / update / postUpdate,
• This at simulation step 200 / 300 / 400 respectively (i.e. not at the same time).

There are two sdf files included:

• one with the boxes on the ground plane (boxes_on_ground.sdf) and
• one with the boxes in the air (boxes_above_ground.sdf).

By renaming the boxes in the sdf it is possible to disable the corresponding remove request.

Running gz sim boxes_on_ground.sdf (i.e. calling RequestRemoveEntity() on a non-moving box) results in all boxes being removed, but the postUpdate box still visible in the gui. I assume this is related to, and possibly fixed by https://github.com/gazebosim/gz-sim/pull/2010.

However running gz sim boxes_above_ground.sdf (i.e. calling RequestRemoveEntity() on a moving box) consistently results in a segmentation fault both for the update and postUpdate boxes:

#4    Object "/opt/gazebo_garden/install/lib/libgz-sim7.so.7", at 0x7fb0fd41ec49, in gz::sim::v7::SimulationRunner::UpdateSystems()
#3    Object "/opt/gazebo_garden/install/lib/gz-sim-7/plugins/libgz-sim-physics-system.so", at 0x7fb0f45391ca, in gz::sim::v7::systems::Physics::Update(gz::sim::v7::UpdateInfo const&, gz::sim::v7::EntityComponentManager&)
#2    Object "/opt/gazebo_garden/install/lib/gz-sim-7/plugins/libgz-sim-physics-system.so", at 0x7fb0f45377e5, in gz::sim::v7::systems::PhysicsPrivate::UpdateSim(gz::sim::v7::EntityComponentManager&, std::map<unsigned long, gz::physics::FrameData<double, 3ul>, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, gz::physics::FrameData<double, 3ul> > > >&)
#1    Object "/opt/gazebo_garden/install/lib/gz-sim-7/plugins/libgz-sim-physics-system.so", at 0x7fb0f4536899, in gz::sim::v7::systems::PhysicsPrivate::UpdateModelPose(unsigned long, unsigned long, gz::sim::v7::EntityComponentManager&, std::map<unsigned long, gz::physics::FrameData<double, 3ul>, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, gz::physics::FrameData<double, 3ul> > > >&)
#0    Object "/opt/gazebo_garden/install/lib/gz-sim-7/plugins/libgz-sim-physics-system.so", at 0x7fb0f455ae74, in std::_Hashtable<unsigned long, std::pair<unsigned long const, gz::math::v7::Pose3<double> >, std::allocator<std::pair<unsigned long const, gz::math::v7::Pose3<double> > >, std::__detail::_Select1st, std::equal_to<unsigned long>, std::hash<unsigned long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::find(unsigned long const&)
Segmentation fault (Address not mapped to object [0x8])

[azeey]

Part of the problem is SdfEntityCreator keeps it's own mutable pointer to the ECM and can technically make calls to it at any system phase, but the PostUpdate gets a const ECM signalling to users that the ECM should not be modified in this phase. @mjcarroll, here's a data point where keeping a pointer to the ECM could be bad.

I'm not sure why it segfaults when called from Update. Generally, the Update phase is reserved for the physics system, but that's not a strict requirement . The ECM should be mutable and systems are run sequentially. This might be a bug in the way we handle removed entities int he Physics system