Merge pull request #94 from juanjux/fix/leak2

Memleaks and other fixes

bblfsh/memtracker.cc

@@ -16,35 +16,20 @@ void MemTracker::TrackItem(PyObject *o)
   }
 }
 
-void MemTracker::TrackStr(PyObject *o)
-{
-  if (inFilter_) {
-    filterStrAllocs_.push_back(o);
-  } else {
-    iterStrAllocs_[currentIter_].push_back(o);
-  }
-}
-
 void MemTracker::DisposeMem()
 {
   if (inFilter_) {
-    for (auto &i : filterStrAllocs_) {
-      Py_XDECREF(i);
-      i = nullptr;
-    }
     for (auto &i : filterItemAllocs_) {
-      Py_XDECREF(i);
-      i = nullptr;
+      Py_CLEAR(i);
     }
+    filterItemAllocs_.clear();
+    filterItemAllocs_.shrink_to_fit();
   } else {
-    for (auto &i : iterStrAllocs_[currentIter_]) {
-      Py_XDECREF(i);
-      i = nullptr;
-    }
     for (auto &i : iterItemAllocs_[currentIter_]) {
-      Py_XDECREF(i);
-      i = nullptr;
+      Py_CLEAR(i);
     }
+    iterItemAllocs_[currentIter_].clear();
+    iterItemAllocs_.erase(currentIter_);
     ClearCurrentIterator();
   }
 }

bblfsh/memtracker.h

@@ -11,9 +11,7 @@ class MemTracker {
   bool inFilter_ = false;
 
   std::unordered_map<UastIterator*, std::vector<PyObject*>> iterItemAllocs_;
-  std::unordered_map<UastIterator*, std::vector<PyObject*>> iterStrAllocs_;
   std::vector<PyObject*> filterItemAllocs_;
-  std::vector<PyObject*> filterStrAllocs_;
 
 public:
   UastIterator *CurrentIterator();
@@ -23,6 +21,5 @@ class MemTracker {
   void EnterFilter();
   void ExitFilter();
   void TrackItem(PyObject *ref);
-  void TrackStr(PyObject *ref);
   void DisposeMem();
 };

bblfsh/pyuast.cc

@@ -11,7 +11,7 @@
 MemTracker memTracker;
 
 // Used to store references to the Pyobjects instanced in String() and
-// ItemAt() methods. Those can't be DECREF'ed to 0 because libuast uses the
+// ItemAt() methods. Those can't be DECREF'ed to 0 because libuast uses them
 // so we pass ownership to these lists and free them at the end of filter()
 
 static PyObject *Attribute(const void *node, const char *prop) {
@@ -29,7 +29,7 @@ static const char *String(const void *node, const char *prop) {
   PyObject *o = Attribute(node, prop);
   if (o != NULL) {
     retval = PyUnicode_AsUTF8(o);
-    memTracker.TrackStr(o);
+    memTracker.TrackItem(o);
   }
   return retval;
 }
@@ -39,6 +39,7 @@ static size_t Size(const void *node, const char *prop) {
   PyObject *o = Attribute(node, prop);
   if (o != NULL) {
     retval = PySequence_Size(o);
+    Py_DECREF(o);
   }
 
   return retval;
@@ -69,21 +70,37 @@ static size_t ChildrenSize(const void *node) {
 
 static void *ChildAt(const void *node, int index) {
   PyObject *children = AttributeValue(node, "children");
-  return children ? ItemAt(children, index) : NULL;
+  void *retval = nullptr;
+  if (children) {
+    retval = ItemAt(children, index);
+    Py_DECREF(children);
+  }
+
+  return retval;
 }
 
 static size_t RolesSize(const void *node) {
   return Size(node, "roles");
 }
 
 static uint16_t RoleAt(const void *node, int index) {
+  uint16_t retval = 0;
   PyObject *roles = AttributeValue(node, "roles");
-  return roles ? (uint16_t)PyLong_AsUnsignedLong(ItemAt(roles, index)) : 0;
+  if (roles) {
+    retval = (uint16_t)PyLong_AsUnsignedLong(ItemAt(roles, index));
+    Py_DECREF(roles);
+  }
+  return retval;
 }
 
 static size_t PropertiesSize(const void *node) {
+  size_t retval = 0;
   PyObject *properties = AttributeValue(node, "properties");
-  return properties ? PyMapping_Size(properties) : 0;
+  if (properties) {
+    retval = PyMapping_Size(properties);
+    Py_DECREF(properties);
+  }
+  return retval;
 }
 
 static const char *PropertyKeyAt(const void *node, int index) {
@@ -94,6 +111,7 @@ static const char *PropertyKeyAt(const void *node, int index) {
 
   const char *retval = NULL;
   PyObject *keys = PyMapping_Keys(properties);
+  Py_DECREF(properties);
   if (keys != NULL) {
     retval = PyUnicode_AsUTF8(ItemAt(keys, index));
     Py_DECREF(keys);
@@ -103,7 +121,11 @@ static const char *PropertyKeyAt(const void *node, int index) {
 
 static const char *PropertyValueAt(const void *node, int index) {
   PyObject *properties = AttributeValue(node, "properties");
-  if (!properties || !PyMapping_Check(properties)) {
+  if (!properties)
+    return NULL;
+
+  if (!PyMapping_Check(properties)) {
+    Py_DECREF(properties);
     return NULL;
   }
 
@@ -113,6 +135,7 @@ static const char *PropertyValueAt(const void *node, int index) {
     retval = PyUnicode_AsUTF8(ItemAt(values, index));
     Py_DECREF(values);
   }
+  Py_DECREF(properties);
   return retval;
 }
 
@@ -123,7 +146,14 @@ static uint32_t PositionValue(const void* node, const char *prop, const char *fi
   }
 
   PyObject *offset = AttributeValue(position, field);
-  return offset ? (uint32_t)PyLong_AsUnsignedLong(offset) : 0;
+  Py_DECREF(position);
+  uint32_t retval = 0;
+
+  if (offset) {
+    retval = (uint32_t)PyLong_AsUnsignedLong(offset);
+    Py_DECREF(offset);
+  }
+  return retval;
 }
 
 /////////////////////////////////////
@@ -337,6 +367,7 @@ static PyObject *PyFilter(PyObject *self, PyObject *args)
     cleanupFilter();
     return NULL;
   }
+
   size_t len = NodesSize(nodes);
   PyObject *list = PyList_New(len);
 

bblfsh/test.py

@@ -252,7 +252,7 @@ def testManyFilters(self):
 
         import resource
         before = resource.getrusage(resource.RUSAGE_SELF)
-        for _ in range(100):
+        for _ in range(500):
             filter(root, "//*[@roleIdentifier]")
         after = resource.getrusage(resource.RUSAGE_SELF)
 

setup.py

@@ -1,16 +1,20 @@
 import os
-import subprocess
 import sys
 
 from setuptools import setup, find_packages, Extension
 from setuptools.command.build_ext import build_ext
 
-VERSION = "2.9.6"
+VERSION = "2.9.10"
 LIBUAST_VERSION = "v1.9.1"
 SDK_VERSION = "v1.8.0"
 SDK_MAJOR = SDK_VERSION.split('.')[0]
 PYTHON = "python3"
 
+# For debugging libuast-client interactions, set to True in production!
+GET_LIBUAST = True
+if not GET_LIBUAST:
+    print('WARNING: not retrieving libuast, using local version')
+
 os.environ["CC"] = "g++"
 os.environ["CXX"] = "g++"
 libraries = ['xml2']
@@ -69,6 +73,9 @@ def createInits():
 
 
 def getLibuast():
+    if not GET_LIBUAST:
+        return
+
     runc("curl -SL https://github.com/bblfsh/libuast/releases/download/{LIBUAST_VERSION}/"
          "libuast-{LIBUAST_VERSION}.tar.gz | tar xz")
     runc("mv libuast-{LIBUAST_VERSION} libuast")
@@ -108,7 +115,8 @@ def clean():
     runc("rm -rf gopkg.in")
     runc("rm -rf bblfsh/github")
     runc("rm -rf bblfsh/gopkg")
-    runc("rm -rf bblfsh/libuast")
+    if GET_LIBUAST:
+        runc("rm -rf bblfsh/libuast")
 
 
 def main():
@@ -153,7 +161,6 @@ def main():
             "Intended Audience :: Developers",
             "License :: OSI Approved :: Apache Software License",
             "Operating System :: POSIX",
-            "Programming Language :: Python :: 2.7",
             "Programming Language :: Python :: 3.4",
             "Programming Language :: Python :: 3.5",
             "Programming Language :: Python :: 3.6",