Fix segfault when vtkObject observed in python is deleted after interpreter is "ended"

* Added a regression test to illustrate the problem

* The solution consisted in using "Py_Finalize" instead of "Py_EndInterpreter".
This is required because within the method vtkPythonCommand::Execute, the method
is a no-op if "Py_IsInitialized" returns 0.

Applications/ctkSimplePythonShell/CMakeLists.txt

@@ -22,8 +22,9 @@ set(KIT_SRCS
 if(CTK_LIB_Scripting/Python/Core_PYTHONQT_USE_VTK)
   list(APPEND KIT_SRCS
     ctkTestWrappedQListOfVTKObject.h
-    ctkTestWrappedVTKSlot.h
+    ctkTestWrappedVTKObserver.h
     ctkTestWrappedVTKQInvokable.h
+    ctkTestWrappedVTKSlot.h
     )
 endif()
 
@@ -39,8 +40,9 @@ set(KIT_MOC_SRCS
 if(CTK_LIB_Scripting/Python/Core_PYTHONQT_USE_VTK)
   list(APPEND KIT_MOC_SRCS
     ctkTestWrappedQListOfVTKObject.h
-    ctkTestWrappedVTKSlot.h
+    ctkTestWrappedVTKObserver.h
     ctkTestWrappedVTKQInvokable.h
+    ctkTestWrappedVTKSlot.h
     )
 endif()
 

Applications/ctkSimplePythonShell/Testing/Python/CMakeLists.txt

@@ -21,6 +21,7 @@ endif()
 if(CTK_LIB_Scripting/Python/Core_PYTHONQT_USE_VTK)
   list(APPEND SCRIPTS
     vtkPythonSmoke.py
+    wrappedVTKObserverTest.py
     wrappedVTKQInvokableTest.py
     wrappedVTKSlotTest.py
     )

Applications/ctkSimplePythonShell/Testing/Python/wrappedVTKObserverTest.py

@@ -0,0 +1,17 @@
+
+import qt
+
+# Importing vtk initializes vtkPythonMap owned by vtkPythonUtil and prevent
+# call to vtkPythonUtil::GetObjectFromPointer() from segfaulting.
+# PythonQt internally uses vtkPythonUtil to properly wrap/unwrap VTK objects
+from vtk import *
+
+t = _testWrappedVTKObserverInstance.getTable()
+
+def onTableModified(caller, event):
+    print("Table modified !")
+
+t.AddObserver(vtkCommand.ModifiedEvent, onTableModified)
+t.Modified()
+
+qt.QApplication.exit(0)

Applications/ctkSimplePythonShell/ctkSimplePythonShellMain.cpp

@@ -36,8 +36,9 @@
 
 #ifdef CTK_WRAP_PYTHONQT_USE_VTK
 # include "ctkTestWrappedQListOfVTKObject.h"
-# include "ctkTestWrappedVTKSlot.h"
+# include "ctkTestWrappedVTKObserver.h"
 # include "ctkTestWrappedVTKQInvokable.h"
+# include "ctkTestWrappedVTKSlot.h"
 # include <vtkDebugLeaks.h>
 #endif
 
@@ -65,80 +66,90 @@ int main(int argc, char** argv)
 {
 #ifdef CTK_WRAP_PYTHONQT_USE_VTK
   vtkDebugLeaks::SetExitError(true);
+  ctkTestWrappedVTKObserver testWrappedVTKObserver;
 #endif
 
-  QApplication app(argc, argv);
+  int exitCode = EXIT_FAILURE;
+  {
+    QApplication app(argc, argv);
 
-  ctkCommandLineParser parser;
-  // Use Unix-style argument names
-  parser.setArgumentPrefix("--", "-");
+    ctkCommandLineParser parser;
+    // Use Unix-style argument names
+    parser.setArgumentPrefix("--", "-");
 
-  // Add command line argument names
-  parser.addArgument("help", "h", QVariant::Bool, "Print usage information and exit.");
-  parser.addArgument("interactive", "I", QVariant::Bool, "Enable interactive mode");
+    // Add command line argument names
+    parser.addArgument("help", "h", QVariant::Bool, "Print usage information and exit.");
+    parser.addArgument("interactive", "I", QVariant::Bool, "Enable interactive mode");
 
-  // Parse the command line arguments
-  bool ok = false;
-  QHash<QString, QVariant> parsedArgs = parser.parseArguments(QCoreApplication::arguments(), &ok);
-  if (!ok)
-  {
-    QTextStream(stderr, QIODevice::WriteOnly) << "Error parsing arguments: "
-                                              << parser.errorString() << "\n";
-    return EXIT_FAILURE;
-  }
+    // Parse the command line arguments
+    bool ok = false;
+    QHash<QString, QVariant> parsedArgs = parser.parseArguments(QCoreApplication::arguments(), &ok);
+    if (!ok)
+    {
+      QTextStream(stderr, QIODevice::WriteOnly) << "Error parsing arguments: "
+                                                << parser.errorString() << "\n";
+      return EXIT_FAILURE;
+    }
 
-  // Show a help message
-  if (parsedArgs.contains("help") || parsedArgs.contains("h"))
-  {
-    QTextStream(stdout, QIODevice::WriteOnly) << "ctkSimplePythonShell\n"
-          << "Usage\n\n"
-          << "  ctkSimplePythonShell [options] [<path-to-python-script> ...]\n\n"
-          << "Options\n"
-          << parser.helpText();
-    return EXIT_SUCCESS;
-  }
+    // Show a help message
+    if (parsedArgs.contains("help") || parsedArgs.contains("h"))
+    {
+      QTextStream(stdout, QIODevice::WriteOnly) << "ctkSimplePythonShell\n"
+            << "Usage\n\n"
+            << "  ctkSimplePythonShell [options] [<path-to-python-script> ...]\n\n"
+            << "Options\n"
+            << parser.helpText();
+      return EXIT_SUCCESS;
+    }
 
-  ctkSimplePythonManager pythonManager;
+    ctkSimplePythonManager pythonManager;
 
-  ctkPythonConsole console;
-  console.initialize(&pythonManager);
-  console.setAttribute(Qt::WA_QuitOnClose, true);
-  console.resize(600, 280);
-  console.show();
+    ctkPythonConsole console;
+    console.initialize(&pythonManager);
+    console.setAttribute(Qt::WA_QuitOnClose, true);
+    console.resize(600, 280);
+    console.show();
 
-  console.setProperty("isInteractive", parsedArgs.contains("interactive"));
+    console.setProperty("isInteractive", parsedArgs.contains("interactive"));
 
-  QStringList list;
-  list << "qt.QPushButton";
-  console.completer()->setAutocompletePreferenceList(list);
+    QStringList list;
+    list << "qt.QPushButton";
+    console.completer()->setAutocompletePreferenceList(list);
 
-  pythonManager.addObjectToPythonMain("_ctkPythonConsoleInstance", &console);
+    pythonManager.addObjectToPythonMain("_ctkPythonConsoleInstance", &console);
 
-  ctkTestWrappedQProperty testWrappedQProperty;
-  pythonManager.addObjectToPythonMain("_testWrappedQPropertyInstance", &testWrappedQProperty);
+    ctkTestWrappedQProperty testWrappedQProperty;
+    pythonManager.addObjectToPythonMain("_testWrappedQPropertyInstance", &testWrappedQProperty);
 
-  ctkTestWrappedQInvokable testWrappedQInvokable;
-  pythonManager.addObjectToPythonMain("_testWrappedQInvokableInstance", &testWrappedQInvokable);
+    ctkTestWrappedQInvokable testWrappedQInvokable;
+    pythonManager.addObjectToPythonMain("_testWrappedQInvokableInstance", &testWrappedQInvokable);
 
-  ctkTestWrappedSlot testWrappedSlot;
-  pythonManager.addObjectToPythonMain("_testWrappedSlotInstance", &testWrappedSlot);
+    ctkTestWrappedSlot testWrappedSlot;
+    pythonManager.addObjectToPythonMain("_testWrappedSlotInstance", &testWrappedSlot);
 
-#ifdef CTK_WRAP_PYTHONQT_USE_VTK
-  ctkTestWrappedVTKQInvokable testWrappedVTKQInvokable;
-  pythonManager.addObjectToPythonMain("_testWrappedVTKQInvokableInstance", &testWrappedVTKQInvokable);
+  #ifdef CTK_WRAP_PYTHONQT_USE_VTK
+    pythonManager.addObjectToPythonMain("_testWrappedVTKObserverInstance", &testWrappedVTKObserver);
 
-  ctkTestWrappedVTKSlot testWrappedVTKSlot;
-  pythonManager.addObjectToPythonMain("_testWrappedVTKSlotInstance", &testWrappedVTKSlot);
+    ctkTestWrappedVTKQInvokable testWrappedVTKQInvokable;
+    pythonManager.addObjectToPythonMain("_testWrappedVTKQInvokableInstance", &testWrappedVTKQInvokable);
 
-  ctkTestWrappedQListOfVTKObject testWrappedQListOfVTKObject;
-  pythonManager.addObjectToPythonMain("_testWrappedQListOfVTKObjectInstance", &testWrappedQListOfVTKObject);
-#endif
+    ctkTestWrappedVTKSlot testWrappedVTKSlot;
+    pythonManager.addObjectToPythonMain("_testWrappedVTKSlotInstance", &testWrappedVTKSlot);
 
-  ctkCallback callback;
-  callback.setCallbackData(&pythonManager);
-  pythonManager.setProperty("scripts", parser.unparsedArguments());
-  callback.setCallback(executeScripts);
-  QTimer::singleShot(0, &callback, SLOT(invoke()));
+  //  ctkTestWrappedQListOfVTKObject testWrappedQListOfVTKObject;
+  //  pythonManager.addObjectToPythonMain("_testWrappedQListOfVTKObjectInstance", &testWrappedQListOfVTKObject);
+  #endif
 
-  return app.exec();
+    ctkCallback callback;
+    callback.setCallbackData(&pythonManager);
+    pythonManager.setProperty("scripts", parser.unparsedArguments());
+    callback.setCallback(executeScripts);
+    QTimer::singleShot(0, &callback, SLOT(invoke()));
+
+    exitCode = app.exec();
+  }
+#ifdef CTK_WRAP_PYTHONQT_USE_VTK
+  testWrappedVTKObserver.getTable()->Modified();
+#endif
+  return exitCode;
 }

Applications/ctkSimplePythonShell/ctkTestWrappedVTKObserver.h

@@ -0,0 +1,57 @@
+/*=========================================================================
+
+  Library:   CTK
+
+  Copyright (c) Kitware Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0.txt
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+=========================================================================*/
+
+#ifndef __ctkTestWrappedVTKObserver_h
+#define __ctkTestWrappedVTKObserver_h
+
+// Qt includes
+#include <QObject>
+
+// VTK includes
+#include <vtkSmartPointer.h>
+#include <vtkTable.h>
+
+class ctkTestWrappedVTKObserver : public QObject
+{
+  Q_OBJECT
+public:
+
+  ctkTestWrappedVTKObserver(QObject * newParent = 0) : QObject(newParent)
+    {
+    this->MyTable = vtkSmartPointer<vtkTable>::New();
+    }
+
+  virtual ~ctkTestWrappedVTKObserver()
+    {
+    }
+
+public Q_SLOTS:
+
+  /// Example of slot returning a VTK object
+  vtkTable* getTable() const
+    {
+    return this->MyTable;
+    }
+
+private:
+  vtkSmartPointer<vtkTable> MyTable;
+};
+
+#endif

Libs/Scripting/Python/Core/ctkAbstractPythonManager.cpp

@@ -82,8 +82,7 @@ ctkAbstractPythonManager::~ctkAbstractPythonManager()
 {
   if (Py_IsInitialized())
     {
-    PyThreadState* state = PyThreadState_Get();
-    Py_EndInterpreter(state);
+    Py_Finalize();
     }
   PythonQt::cleanup();
 }