A gazebo model which uses yarprobotinterface plugin crashes when the model is removed

[randaz81]

As discussed with @traversaro, the reason is probably due to the fact the low-level gazebo plugins are destroyed before yarprobotinerface plugins (i.e. a running NWS cannot access anymore the attached low-level plugin).

Instead, the correct order should be:
1- The user removes the model.
2- The yarprobotinterface plugin initializes the shutdown sequence.
3- All yarprobotinterface plugins are detached and destroyed.
4- The yarprobotinterface plugin is destroyed.
5- All other low-level gazebo plugins which run externally to yarprobotinterface are destroyed.

This hypothesis should be verified by checking in the gazebo source code the order in which the engine destroys its own plugins.

[traversaro]

At a first glance, indeed it seems that Gazebo stores the plugin shared pointers in Model::plugins data structure, see:

• https://github.com/osrf/gazebo/blob/135a8c1fd9f28e18c0daa171bc82ad9afdb9883d/gazebo/physics/Model.hh#L545
• https://github.com/osrf/gazebo/blob/135a8c1fd9f28e18c0daa171bc82ad9afdb9883d/gazebo/physics/Model.cc#L1187

During model destruction, there is no attempt to destroy the plugin in reverse order w.r.t. to loading order:

• https://github.com/osrf/gazebo/blob/135a8c1fd9f28e18c0daa171bc82ad9afdb9883d/gazebo/physics/Model.cc#L69
• https://github.com/osrf/gazebo/blob/135a8c1fd9f28e18c0daa171bc82ad9afdb9883d/gazebo/physics/Model.cc#L520

[traversaro]

Probably just adding in Gazebo a for-loop that calls reset in the correct order for the Model::plugins vector will be enough in most cases. That will not work if somebody else saved the shared_ptr to the gazebo_yarp_robotinterface, but I guess this is a rather peculiar case that will not happen in practice.

[traversaro]

This is affecting stickBot, @Nicogene do you have any idea if someone ever complained about it?

[Nicogene]

This is affecting stickBot, @Nicogene do you have any idea if someone ever complained about it?

Nobody complained about it but it happens always I think. it's pretty deterministic

[traversaro]

@lrapetti @mebbaid this is not problematic for your workflows?

[mebbaid]

Indeed it would be much better if I didn't have to restart both gazebo and the yarprobotinterface everytime , if I understood this correctly.

[traversaro]

So you noticed it, right? This is not happening at the moment with normal icub models, but it may happen if we fix robotology/icub-models-generator#215 without first solving this problem, that why I am asking.

[mebbaid]

Yeah I confirm that I noticed it upong model removal Everytime.

[traversaro]

I think I found a solution that does not require any modification in upstream Gazebo. Basically the gazebo_yarp_robotinterface plugin should register a call in the singleton object that stores the pointer to all created devices to be notified whenever a device is removed. If a device that was passed to the yarprobotinterface is removed, this will trigger the shutdown of the robotinterface, regardless of the order in which the plugin are destroyed.

[traversaro]

This will be fixed by #582 .

[traversaro]

Fixed and released in https://github.com/robotology/gazebo-yarp-plugins/releases/tag/v4.3.0 . Note that you may need to wait for robotology/whole-body-estimators#146 if your model uses wholebodydynamics device, such as some models from ergocub-gazebo-models .