Andreas Sandberg has submitted this change and it was merged. ( https://gem5-review.googlesource.com/3224 )

Change subject: python: Prevent Python wrappers from deleting SimObjects
......................................................................

python: Prevent Python wrappers from deleting SimObjects

The PyBind wrappers could potentially delete SimObjects if they don't
have any references. This is not desirable since there could be
pointers to such objects within the C++ world. This problem doesn't
normally occur since Python typically holds a pointer to the root node
as long as the simulator is running.

Prevent SimObject and Param deletion by using a PyBind-prescribed
unique_ptr with a dummy deleter as the pointer wrapper for the Python
world.

Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Signed-off-by: Andreas Sandberg <[email protected]>
Reviewed-by: Curtis Dunham <[email protected]>
Reviewed-on: https://gem5-review.googlesource.com/3224
Reviewed-by: Gabe Black <[email protected]>
---
M src/python/m5/SimObject.py
M src/python/pybind11/core.cc
2 files changed, 15 insertions(+), 8 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved
  Andreas Sandberg: Looks good to me, approved



diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index 569142e..baeef73 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -701,10 +701,13 @@
 ''')
         code.indent()
         if cls._base:
- code('py::class_<${cls}Params, ${{cls._base.type}}Params>(m, ' \
-                 '"${cls}Params")')
+            code('py::class_<${cls}Params, ${{cls._base.type}}Params, ' \
+                 'std::unique_ptr<${{cls}}Params, py::nodelete>>(' \
+                 'm, "${cls}Params")')
         else:
-            code('py::class_<${cls}Params>(m, "${cls}Params")')
+            code('py::class_<${cls}Params, ' \
+                 'std::unique_ptr<${cls}Params, py::nodelete>>(' \
+                 'm, "${cls}Params")')

         code.indent()
         if not hasattr(cls, 'abstract') or not cls.abstract:
@@ -729,10 +732,13 @@
                 cls.cxx_bases
         if bases:
             base_str = ", ".join(bases)
-            code('py::class_<${{cls.cxx_class}}, ${base_str}>(m, ' \
-                 '"${py_class_name}")')
+            code('py::class_<${{cls.cxx_class}}, ${base_str}, ' \
+                 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+                 'm, "${py_class_name}")')
         else:
-            code('py::class_<${{cls.cxx_class}}>(m, "${py_class_name}")')
+            code('py::class_<${{cls.cxx_class}}, ' \
+                 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+                 'm, "${py_class_name}")')
         code.indent()
         for exp in cls.cxx_exports:
             exp.export(code, cls.cxx_class)
diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 7ad45b4..159b19f 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -132,7 +132,8 @@
 {
     py::module m = m_native.def_submodule("serialize");

-    py::class_<Serializable>(m, "Serializable")
+    py::class_<Serializable, std::unique_ptr<Serializable, py::nodelete>>(
+        m, "Serializable")
         ;

     py::class_<CheckpointIn>(m, "CheckpointIn")
@@ -165,7 +166,7 @@
         .def("isSubset", &AddrRange::isSubset)
         ;

-    // We need to make vectors of AddrRange opaque to avoid a weird
+    // We need to make vectors of AddrRange opaque to avoid weird
     // memory allocation issues in PyBind's STL wrappers.
     py::bind_vector<std::vector<AddrRange>>(m, "AddrRangeVector");


--
To view, visit https://gem5-review.googlesource.com/3224
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Gerrit-Change-Number: 3224
Gerrit-PatchSet: 5
Gerrit-Owner: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Curtis Dunham <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to