Hello Curtis Dunham,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/3224
to review the following change.
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]>
---
M src/python/m5/SimObject.py
M src/python/pybind11/core.cc
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index b621a82..2243307 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -699,10 +699,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:
@@ -727,10 +730,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: newchange
Gerrit-Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Gerrit-Change-Number: 3224
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Curtis Dunham <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
