Python object reference counting is incorrect (#2400)

* factor out ref counting in Python Invocation * tighten up reference counting in python embed * whitespace normalization
PDAL · Mar 11, 2019 · 496edc9 · 496edc9
1 parent c9dbb39
commit 496edc9
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 73 deletions.
diff --git a/plugins/python/filters/PythonFilter.cpp b/plugins/python/filters/PythonFilter.cpp
@@ -78,7 +78,8 @@ void PythonFilter::ready(PointTableRef table)
         m_source = FileUtils::readFileIntoString(m_scriptFile);
     static_cast<plang::Environment*>(plang::Environment::get())->set_stdout(log()->getLogStream());
     m_script = new plang::Script(m_source, m_module, m_function);
-    m_pythonMethod = new plang::Invocation(*m_script);
+
+	m_pythonMethod = new plang::Invocation(*m_script);
     m_pythonMethod->compile();
     m_totalMetadata = table.metadata();
 }

diff --git a/plugins/python/plang/Environment.cpp b/plugins/python/plang/Environment.cpp
@@ -59,7 +59,7 @@
 // __attribute__ ((constructor)) this does nothing.  We'll have to deal with
 // those as they come up.
 #ifndef _WIN32
-__attribute__ ((constructor))
+__attribute__((constructor))
 static void loadPython()
 {
     ::dlopen(PDAL_PYTHON_LIBRARY, RTLD_LAZY | RTLD_GLOBAL);
@@ -74,8 +74,7 @@ namespace pdal
 {
 namespace plang
 {
-
-static Environment* g_environment=0;
+static Environment* g_environment = 0;
 //
 EnvironmentPtr Environment::get()
 {
@@ -108,10 +107,11 @@ Environment::Environment()
     {
         PyImport_AppendInittab(const_cast<char*>("redirector"), redirector_init);
         Py_Initialize();
-    } else
+    }
+    else
     {
         m_redirector.init();
-        PyObject* added  = PyImport_AddModule("redirector");
+        PyObject* added = PyImport_AddModule("redirector");
         if (!added)
             throw pdal_error("unable to add redirector module!");
     }
@@ -120,25 +120,21 @@ Environment::Environment()
     PyImport_ImportModule("redirector");
 }
 
-
 Environment::~Environment()
 {
     Py_Finalize();
 }
 
-
 void Environment::set_stdout(std::ostream* ostr)
 {
     m_redirector.set_stdout(ostr);
 }
 
-
 void Environment::reset_stdout()
 {
     m_redirector.reset_stdout();
 }
 
-
 std::string getTraceback()
 {
     // get exception info
@@ -216,12 +212,12 @@ std::string getTraceback()
     }
     else
         mssg << "unknown error that we are unable to get a traceback for."
-            "Was it already printed/taken?";
+        "Was it already printed/taken?";
 
     Py_XDECREF(value);
     Py_XDECREF(type);
     Py_XDECREF(traceback);
-
+    PyErr_Clear();
     return mssg.str();
 }
 
@@ -233,10 +229,10 @@ PyObject *fromMetadata(MetadataNode m)
     std::string description = m.description();
 
     MetadataNodeList children = m.children();
-	PyObject *submeta(0);
+    PyObject *submeta(0);
     if (children.size())
     {
-		submeta = PyList_New(0);
+        submeta = PyList_New(0);
         for (MetadataNode& child : children)
             PyList_Append(submeta, fromMetadata(child));
     }
@@ -245,10 +241,10 @@ PyObject *fromMetadata(MetadataNode m)
     PyDict_SetItemString(data, "value", PyUnicode_FromString(value.data()));
     PyDict_SetItemString(data, "type", PyUnicode_FromString(type.data()));
     PyDict_SetItemString(data, "description", PyUnicode_FromString(description.data()));
-	if (children.size())
-	{
-		PyDict_SetItemString(data, "children", submeta);
-	}
+    if (children.size())
+    {
+        PyDict_SetItemString(data, "children", submeta);
+    }
     return data;
 }
 
@@ -279,13 +275,12 @@ std::string readPythonString(PyObject* dict, const std::string& key)
 }
 void addMetadata(PyObject *dict, MetadataNode m)
 {
-
-    if (! dict)
+    if (!dict)
     {
         return;
     }
 
-    if (!PyDict_Check(dict) )
+    if (!PyDict_Check(dict))
         throw pdal::pdal_error("'metadata' member must be a dictionary!");
 
     std::string name = readPythonString(dict, "name");
@@ -308,7 +303,7 @@ void addMetadata(PyObject *dict, MetadataNode m)
             PyObject* p = PyList_GetItem(submeta, i);
             addMetadata(p, m);
         }
-        MetadataNode child =  m.addWithType(name, value, type, description);
+        MetadataNode child = m.addWithType(name, value, type, description);
     }
 }
 
@@ -346,7 +341,6 @@ int Environment::getPythonDataType(Dimension::Type t)
     return -1;
 }
 
-
 Dimension::Type Environment::getPDALDataType(int t)
 {
     using namespace Dimension;
@@ -380,7 +374,5 @@ Dimension::Type Environment::getPDALDataType(int t)
 
     return Type::None;
 }
-
 } // namespace plang
 } // namespace pdal
-