From c8be57af51fbf7c91a5c813211d07c5621ae9889 Mon Sep 17 00:00:00 2001
From: Joseph Mirabel <jmirabel@laas.fr>
Date: Wed, 26 Sep 2018 19:24:32 +0200
Subject: [PATCH] Update Interpreter destructor.

---
 src/interpreter.cc                            | 43 ++++++++++++++++---
 unitTesting/CMakeLists.txt                    |  2 +
 unitTesting/interpreter-test-runfile.cc       | 36 ++++++++++++++--
 .../test_python-restart_interpreter.py        | 10 +++++
 4 files changed, 82 insertions(+), 9 deletions(-)
 create mode 100644 unitTesting/test_python-restart_interpreter.py

diff --git a/src/interpreter.cc b/src/interpreter.cc
index 9abf3c2..b1fc04f 100644
--- a/src/interpreter.cc
+++ b/src/interpreter.cc
@@ -26,7 +26,6 @@
 #include <boost/python/str.hpp>
 #include <boost/python/import.hpp>
 
-using namespace boost::python;
 namespace py=boost::python;
 
 std::ofstream dg_debugfile( "/tmp/dynamic-graph-traces.txt", std::ios::trunc&std::ios::out );
@@ -164,10 +163,44 @@ Interpreter::Interpreter()
 Interpreter::~Interpreter()
 {
   PyEval_RestoreThread(_pyState);
-  //Py_DECREF(mainmod_);
-  //Py_DECREF(globals_);
-  //Py_DECREF(traceback_format_exception_);
-  Py_Finalize();
+
+  // Ideally, we should call Py_Finalize but this is not really supported by
+  // Python.
+  // Instead, we merelly remove variables.
+  // Code was taken here: https://github.com/numpy/numpy/issues/8097#issuecomment-356683953
+  {
+    PyObject * poAttrList = PyObject_Dir(mainmod_);
+    PyObject * poAttrIter = PyObject_GetIter(poAttrList);
+    PyObject * poAttrName;
+
+    while ((poAttrName = PyIter_Next(poAttrIter)) != NULL)
+    {
+      std::string oAttrName (PyString_AS_STRING(poAttrName));
+
+      // Make sure we don't delete any private objects.
+      if (oAttrName.compare(                 0,2,"__")!=0 ||
+          oAttrName.compare(oAttrName.size()-2,2,"__")!=0)
+      {
+        PyObject * poAttr = PyObject_GetAttr(mainmod_, poAttrName);
+
+        // Make sure we don't delete any module objects.
+        if (poAttr && poAttr->ob_type != mainmod_->ob_type)
+          PyObject_SetAttr(mainmod_, poAttrName, NULL);
+
+        Py_DecRef(poAttr);
+      }
+
+      Py_DecRef(poAttrName);
+    }
+
+    Py_DecRef(poAttrIter);
+    Py_DecRef(poAttrList);
+  }
+
+  Py_DECREF(mainmod_);
+  Py_DECREF(globals_);
+  Py_DECREF(traceback_format_exception_);
+  //Py_Finalize();
 }
 
 std::string Interpreter::python( const std::string& command )
diff --git a/unitTesting/CMakeLists.txt b/unitTesting/CMakeLists.txt
index 6457f79..212a4c4 100644
--- a/unitTesting/CMakeLists.txt
+++ b/unitTesting/CMakeLists.txt
@@ -32,6 +32,8 @@ ADD_TEST(${EXECUTABLE_NAME}  ${EXECUTABLE_NAME})
 SET(EXECUTABLE_NAME interpreter-test-runfile)
 ADD_EXECUTABLE(${EXECUTABLE_NAME} interpreter-test-runfile.cc)
 TARGET_LINK_LIBRARIES(${EXECUTABLE_NAME} dynamic-graph-python)
+TARGET_LINK_LIBRARIES(${EXECUTABLE_NAME} ${PYTHON_LIBRARY})
+TARGET_LINK_BOOST_PYTHON(${EXECUTABLE_NAME})
 ADD_TEST(${EXECUTABLE_NAME}  ${EXECUTABLE_NAME})
 
 ADD_CUSTOM_COMMAND(TARGET interpreter-test-runfile POST_BUILD
diff --git a/unitTesting/interpreter-test-runfile.cc b/unitTesting/interpreter-test-runfile.cc
index 8b8a854..271e8a5 100644
--- a/unitTesting/interpreter-test-runfile.cc
+++ b/unitTesting/interpreter-test-runfile.cc
@@ -24,6 +24,28 @@ bool testFile(const std::string & filename,
   return true;
 }
 
+bool testInterpreterDestructor(const std::string & filename, 
+             const std::string & expectedOutput)
+{
+  std::string err = "";
+  {
+    dynamicgraph::python::Interpreter interp;
+    interp.runPythonFile(filename, err);
+  }
+  {
+    dynamicgraph::python::Interpreter interp;
+    interp.runPythonFile(filename, err);
+    if (err != expectedOutput)
+    {
+      std::cerr << "The output was not the one expected:" << std::endl;
+      std::cerr << " expected: " << expectedOutput << std::endl;
+      std::cerr << " err:      " << err << std::endl;
+      return false;
+    }
+  }  
+  return true;
+}
+
 int main(int argc, char ** argv)
 {
   // execute numerous time the same file.
@@ -34,17 +56,23 @@ int main(int argc, char ** argv)
     numTest = atoi(argv[1]);
 
   bool res = true;
-  res = testFile("test_python-ok.py", "", numTest) && res;
-  res = testFile("unexistant_file.py", 
-		 "unexistant_file.py cannot be open",
-		 numTest) && res;
+  // This test succeeds only because it is launched before "test_python-ok.py"
+  // because re as been imported in a previous test and it is not
+  // safe to delete imported module...
   res = testFile("test_python-name_error.py", 
 		 std::string("<type 'exceptions.NameError'>: name 're' is not defined:")+
 		 "   File \"test_python-name_error.py\", line 6, in <module>\n" +
 		 "    pathList = re.split(':', pkgConfigPath)\n", numTest) && res;
+
+  res = testFile("test_python-ok.py", "", numTest) && res;
+  res = testFile("unexistant_file.py", 
+		 "unexistant_file.py cannot be open",
+		 numTest) && res;
   res = testFile("test_python-syntax_error.py", 
 		 std::string("<type 'exceptions.SyntaxError'>: ('invalid syntax', ")+
 		 "('test_python-syntax_error.py', 1, 11, "+
 		 "'hello world\\n'))", numTest) && res;
+  res = testInterpreterDestructor ("test_python-restart_interpreter.py",
+                                   "") && res;
   return (res?0:1);
 }
diff --git a/unitTesting/test_python-restart_interpreter.py b/unitTesting/test_python-restart_interpreter.py
new file mode 100644
index 0000000..a2cf3ef
--- /dev/null
+++ b/unitTesting/test_python-restart_interpreter.py
@@ -0,0 +1,10 @@
+# this causes troubles when Py_Finalize is called in Interpreter destructor.
+import dynamic_graph.sot.core
+# numpy causes troubles when Py_Finalize is called in Interpreter destructor.
+import numpy
+
+# Make sure the variable is deleted.
+if "var" in locals() or "var" in globals():
+    raise ValueError('Not cleaned')
+
+var = "This string should have been deleted."
-- 
GitLab