Added gil acquire to Configure, Destructor, and Reset methods. Also used the TestModelSystem plugin for testing

Signed-off-by: Amal Dev Haridevan <[email protected]>

Author: AmalDevHaridevan

python/src/gz/sim/TestFixture.cc

@@ -56,14 +56,7 @@ defineSimTestFixture(pybind11::object module)
       [](TestFixture* self, std::function<void(
           const UpdateInfo &, EntityComponentManager &)> _cb)
       {
-        // Add explicit scoped acquire and release of GIL, so that Python Systems
-        // can be executed
-        // This acquire and release is only required from the PythonSystem code
-        // However, adding this here may prevent undefined or unintended behaviors
-        // in future
-        pybind11::gil_scoped_acquire gil;
         self->OnPreUpdate(_cb);
-        pybind11::gil_scoped_release gilr;
       }
     ),
     pybind11::return_value_policy::reference,
@@ -74,9 +67,7 @@ defineSimTestFixture(pybind11::object module)
       [](TestFixture* self, std::function<void(
           const UpdateInfo &, EntityComponentManager &)> _cb)
       {
-        pybind11::gil_scoped_acquire gil;
         self->OnUpdate(_cb);
-        pybind11::gil_scoped_release gilr;
       }
     ),
     pybind11::return_value_policy::reference,
@@ -87,9 +78,7 @@ defineSimTestFixture(pybind11::object module)
       [](TestFixture* self, std::function<void(
           const UpdateInfo &, const EntityComponentManager &)> _cb)
       {
-        pybind11::gil_scoped_acquire gil;
         self->OnPostUpdate(_cb);
-        pybind11::gil_scoped_release gilr;
       }
     ),
     pybind11::return_value_policy::reference,

python/test/gravity.sdf

@@ -19,7 +19,7 @@
       </link>
       <plugin filename="gz-sim-python-system-loader-system"
         name="gz::sim::systems::PythonSystemLoader">
-        <module_name>python_system_TEST</module_name>
+        <module_name>plugins.test_model_system</module_name>
       </plugin>
     </model>
 

src/systems/python_system_loader/PythonSystemLoader.cc

@@ -43,6 +43,7 @@ PythonSystemLoader::~PythonSystemLoader()
 {
   if (this->pythonSystem)
   {
+    py::gil_scoped_acquire gil;
     if (py::hasattr(this->pythonSystem, "shutdown"))
     {
       this->pythonSystem.attr("shutdown")();
@@ -54,6 +55,7 @@ void PythonSystemLoader::Configure(
     const Entity &_entity, const std::shared_ptr<const sdf::Element> &_sdf,
     EntityComponentManager &_ecm, EventManager &_eventMgr)
 {
+  py::gil_scoped_acquire gil;
   auto [moduleName, hasModule] = _sdf->Get<std::string>("module_name", "");
   if (!hasModule)
   {
@@ -199,7 +201,6 @@ void PythonSystemLoader::PreUpdate(const UpdateInfo &_info,
   // from the PythonSystem code
   py::gil_scoped_acquire gil;
   CallPythonMethod(this->preUpdateMethod, _info, &_ecm);
-  py::gil_scoped_release gilr;
 }
 
 //////////////////////////////////////////////////
@@ -208,7 +209,6 @@ void PythonSystemLoader::Update(const UpdateInfo &_info,
 {
   py::gil_scoped_acquire gil;
   CallPythonMethod(this->updateMethod, _info, &_ecm);
-  py::gil_scoped_release gilr;
 }
 
 //////////////////////////////////////////////////
@@ -217,12 +217,12 @@ void PythonSystemLoader::PostUpdate(const UpdateInfo &_info,
 {
   py::gil_scoped_acquire gil;
   CallPythonMethod(this->postUpdateMethod, _info, &_ecm);
-  py::gil_scoped_release gilr;
 }
 //////////////////////////////////////////////////
 void PythonSystemLoader::Reset(const UpdateInfo &_info,
                                EntityComponentManager &_ecm)
 {
+  py::gil_scoped_acquire gil;
   CallPythonMethod(this->resetMethod, _info, &_ecm);
 }