Delete plugins when their corresponding objects are deleted, if their instance belonged the object.

Fixes #2061.

PiperOrigin-RevId: 676402701
Change-Id: Ia4807d57a3763e2100702b7e7ec96c300d0a9cfb

src/user/user_mesh.cc

@@ -269,6 +269,9 @@ void mjCMesh::CopyFromSpec() {
 mjCMesh::~mjCMesh() {
   if (center_) mju_free(center_);
   if (graph_) mju_free(graph_);
+  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+    model->DeleteElement(spec.plugin.instance);
+  }
 }
 
 

src/user/user_model.cc

@@ -516,6 +516,7 @@ void mjCModel::DeleteElement(mjsElement* el) {
   }
 
   if (compiled) {
+    ResetTreeLists();  // in case of a nested delete
     MakeLists(world);
     ProcessLists(/*checkrepeat=*/false);
   }
@@ -586,6 +587,9 @@ void mjCModel::CopyFromSpec() {
 
 // destructor
 mjCModel::~mjCModel() {
+  // do not rebuild lists if we are in the process of deleting the model
+  compiled = false;
+
   // delete kinematic tree and all objects allocated in it
   delete bodies_[0];
 

src/user/user_objects.cc

@@ -984,6 +984,10 @@ mjCBody::~mjCBody() {
   sites.clear();
   cameras.clear();
   lights.clear();
+
+  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+    model->DeleteElement(spec.plugin.instance);
+  }
 }
 
 
@@ -2107,6 +2111,14 @@ mjCGeom::mjCGeom(const mjCGeom& other) {
 
 
 
+mjCGeom::~mjCGeom() {
+  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+    model->DeleteElement(spec.plugin.instance);
+  }
+}
+
+
+
 mjCGeom& mjCGeom::operator=(const mjCGeom& other) {
   if (this != &other) {
     this->spec = other.spec;
@@ -5521,6 +5533,14 @@ mjCActuator::mjCActuator(const mjCActuator& other) {
 
 
 
+mjCActuator::~mjCActuator() {
+  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+    model->DeleteElement(spec.plugin.instance);
+  }
+}
+
+
+
 mjCActuator& mjCActuator::operator=(const mjCActuator& other) {
   if (this != &other) {
     this->spec = other.spec;
@@ -5875,6 +5895,14 @@ mjCSensor::mjCSensor(const mjCSensor& other) {
 
 
 
+mjCSensor::~mjCSensor() {
+  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+    model->DeleteElement(spec.plugin.instance);
+  }
+}
+
+
+
 mjCSensor& mjCSensor::operator=(const mjCSensor& other) {
   if (this != &other) {
     this->spec = other.spec;

src/user/user_objects.h

@@ -511,6 +511,7 @@ class mjCGeom : public mjCGeom_, private mjsGeom {
   mjCGeom(mjCModel* = nullptr, mjCDef* = nullptr);
   mjCGeom(const mjCGeom& other);
   mjCGeom& operator=(const mjCGeom& other);
+  ~mjCGeom();
 
   using mjCBase::name;
   mjsGeom spec;                       // variables set by user
@@ -1456,6 +1457,7 @@ class mjCActuator : public mjCActuator_, private mjsActuator {
   mjCActuator(mjCModel* = nullptr, mjCDef* = nullptr);
   mjCActuator(const mjCActuator& other);
   mjCActuator& operator=(const mjCActuator& other);
+  ~mjCActuator();
 
   mjsActuator spec;
   using mjCBase::name;
@@ -1516,6 +1518,7 @@ class mjCSensor : public mjCSensor_, private mjsSensor {
   mjCSensor(mjCModel*);
   mjCSensor(const mjCSensor& other);
   mjCSensor& operator=(const mjCSensor& other);
+  ~mjCSensor();
 
   mjsSensor spec;
   using mjCBase::name;

test/user/user_api_test.cc

@@ -165,6 +165,57 @@ TEST_F(PluginTest, ActivatePlugin) {
   mj_deleteModel(model);
 }
 
+TEST_F(PluginTest, DeletePlugin) {
+  std::string plugin_name = "mujoco.pid";
+  mjSpec* spec = mj_makeSpec();
+
+  // get slot of requested plugin
+  int plugin_slot = -1;
+  const mjpPlugin* plugin = mjp_getPlugin(plugin_name.c_str(), &plugin_slot);
+  ASSERT_THAT(plugin, NotNull());
+
+  // activated plugin in the slot
+  std::vector<std::pair<const mjpPlugin*, int>> active_plugins;
+  active_plugins.emplace_back(std::make_pair(plugin, plugin_slot));
+  mjs_setActivePlugins(spec, &active_plugins);
+
+  // create body
+  mjsBody* body = mjs_addBody(mjs_findBody(spec, "world"), 0);
+  mjsJoint* joint = mjs_addJoint(body, 0);
+  mjsGeom* geom = mjs_addGeom(body, 0);
+  mjs_setString(joint->name, "j1");
+  joint->type = mjJNT_SLIDE;
+  geom->size[0] = 1;
+
+  // add actuator
+  mjsActuator* actuator = mjs_addActuator(spec, 0);
+  mjs_setString(actuator->target, "j1");
+  mjs_setString(actuator->plugin.name, plugin_name.c_str());
+  actuator->plugin.instance = mjs_addPlugin(spec)->instance;
+  actuator->plugin.active = true;
+  actuator->trntype = mjTRN_JOINT;
+
+  // compile and check that the plugin is present
+  mjModel* model = mj_compile(spec, NULL);
+  EXPECT_THAT(model, NotNull());
+  EXPECT_THAT(model->nu, 1);
+  EXPECT_THAT(model->nplugin, 1);
+  EXPECT_THAT(model->actuator_plugin[0], 0);
+
+  // delete actuator
+  mjs_delete(actuator->element);
+
+  // recompile and check that the plugin is not present
+  mjModel* newmodel = mj_compile(spec, NULL);
+  EXPECT_THAT(newmodel, NotNull());
+  EXPECT_THAT(newmodel->nu, 0);
+  EXPECT_THAT(newmodel->nplugin, 0);
+
+  mj_deleteSpec(spec);
+  mj_deleteModel(model);
+  mj_deleteModel(newmodel);
+}
+
 TEST_F(MujocoTest, RecompileFails) {
   mjSpec* spec = mj_makeSpec();
   mjsBody* body = mjs_addBody(mjs_findBody(spec, "world"), 0);