From d3500ecfabf8b3130fca416689696e7a5f2b327f Mon Sep 17 00:00:00 2001
From: "Zheng, Lei" <realthunder.dev@gmail.com>
Date: Fri, 2 Aug 2019 09:15:49 +0800
Subject: [PATCH] App: fix Python object leak in PropertyListT

* Fix Python object leak in _setPyObject()
* Add support for Python iterables
* Minor performance improvement on setPyValues()
---
 src/App/Property.cpp | 55 ++++++++++++++++++++++++++++----------------
 src/App/Property.h   | 25 ++++++++++----------
 2 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/src/App/Property.cpp b/src/App/Property.cpp
index 29c76f0a04..58ad4b556c 100644
--- a/src/App/Property.cpp
+++ b/src/App/Property.cpp
@@ -213,44 +213,59 @@ void Property::setStatus(Status pos, bool on) {
 //++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 
 void PropertyListsBase::_setPyObject(PyObject *value) {
-    std::vector<PyObject *> vals;
     std::vector<int> indices;
+    std::vector<PyObject *> vals;
+    Py::Object pySeq;
+
     if (PyDict_Check(value)) {
-        PyObject* keyList = PyDict_Keys(value);
-        PyObject* itemList = PyDict_Values(value);
-        Py_ssize_t nSize = PyList_Size(keyList);
-        vals.reserve(nSize);
-        indices.reserve(nSize);
+        Py::Dict dict(value);
+        auto size = dict.size();
+        vals.reserve(size);
+        indices.reserve(size);
         int listSize = getSize();
-        for (Py_ssize_t i=0; i<nSize;++i) {
-            std::string keyStr;
-            PyObject* key = PyList_GetItem(keyList, i);
+        for(auto it=dict.begin();it!=dict.end();++it) {
+            const auto &item = *it;
+            PyObject *key = item.first.ptr();
 #if PY_MAJOR_VERSION < 3
             if(!PyInt_Check(key)) 
 #else
             if(!PyLong_Check(key))
 #endif
-                throw Base::TypeError("expect key type to be integer");
-            auto idx = PyLong_AsLong(key);
+                throw Base::TypeError("expect key type to be interger");
+            long idx = PyLong_AsLong(key);
             if(idx<-1 || idx>listSize) 
-                throw Base::RuntimeError("index out of bound");
+                throw Base::ValueError("index out of bound");
             if(idx==-1 || idx==listSize) {
                 idx = listSize;
                 ++listSize;
             }
             indices.push_back(idx);
-            vals.push_back(PyList_GetItem(itemList,i));
+            vals.push_back(item.second.ptr());
         }
-    }else if (PySequence_Check(value)) {
-        Py_ssize_t nSize = PySequence_Size(value);
-        vals.reserve(nSize);
-        for (Py_ssize_t i=0; i<nSize;++i)
-            vals.push_back(PySequence_GetItem(value, i));
-    }else
-        vals.push_back(value);
+    } else {
+        if (PySequence_Check(value))
+            pySeq = value;
+        else {
+            PyObject *iter = PyObject_GetIter(value);
+            if(iter) {
+                Py::Object pyIter(iter,true);
+                pySeq = Py::asObject(PySequence_Fast(iter,""));
+            } else {
+                PyErr_Clear();
+                vals.push_back(value);
+            }
+        }
+        if(!pySeq.isNone()) {
+            Py::Sequence seq(pySeq);
+            vals.reserve(seq.size());
+            for(auto it=seq.begin();it!=seq.end();++it)
+                vals.push_back((*it).ptr());
+        }
+    }
     setPyValues(vals,indices);
 }
 
+
 //**************************************************************************
 //**************************************************************************
 // PropertyLists
diff --git a/src/App/Property.h b/src/App/Property.h
index 6a6a2b7789..4c45c6397b 100644
--- a/src/App/Property.h
+++ b/src/App/Property.h
@@ -515,20 +515,19 @@ protected:
 
     void setPyValues(const std::vector<PyObject*> &vals, const std::vector<int> &indices) override 
     {
-        ListT values;
-        // old version boost::dynamic_bitset don't have reserve(). What a shame!
-        // values.reserve(vals.size());
-        for(auto item : vals)
-            values.push_back(getPyValue(item));
-        if(indices.empty())
-            setValues(values);
-        else {
-            atomic_change guard(*this);
-            assert(values.size()==indices.size());
-            for(int i=0,count=values.size();i<count;++i)
-                set1Value(indices[i],values[i]);
-            guard.tryInvoke();
+        if(indices.empty()) {
+            ListT values;
+            values.resize(vals.size());
+            for(std::size_t i=0,count=vals.size();i<count;++i)
+                values[i] = getPyValue(vals[i]);
+            setValues(std::move(values));
+            return;
         }
+        assert(vals.size()==indices.size());
+        atomic_change guard(*this);
+        for(int i=0,count=indices.size();i<count;++i)
+            set1Value(indices[i],getPyValue(vals[i]));
+        guard.tryInvoke();
     }
 
     virtual T getPyValue(PyObject *item) const = 0;