[Lldb-commits] [PATCH] D13617: Fix ref-counting of Python objects

[Zachary Turner via lldb-commits]

zturner created this revision.
zturner added a reviewer: clayborg.
zturner added a subscriber: lldb-commits.

PythonObjects were being incorrectly ref-counted.  This problem was pervasive throughout the codebase, leading to an unknown number of memory leaks and potentially use-after-free.  

The issue stems from the fact that Python native methods can either return "borrowed" references or "owned" references.  For the former category, you *must* incref it prior to decrefing it.  And for the latter category, you should not incref it before decrefing it.  This is mostly an issue when a Python C API method returns a `PyObject` to you, but it can also happen with a method accepts a `PyObject`.  Notably, this happens in `PyList_SetItem`, which is documented to "steal" the reference that you give it.  So if you pass something to `PyList_SetItem`, you cannot hold onto it unless you incref it first.  But since this is one of only two exceptions in the entire API, it's confusing and difficult to remember.

Our `PythonObject` class was indiscriminantely increfing every object it received, which means that if you passed it an owned reference, you now have a dangling reference since owned references should not be increfed.  We were doing this in quite a few places.  

There was also a fair amount of manual increfing and decrefing prevalent throughout the codebase, which is easy to get wrong.

This patch solves the problem by first changing the name of `PythonObject` to `PyRef` (so that it is clear from the name what this actually is), and then making any construction of a `PyRef` from a `PyObject` take a flag which indicates whether it is an owned reference or a borrowed reference.  There is no way to construct a `PyRef` without this flag, and it does not offer a default value, forcing the user to make an explicit decision every time.

All manual uses of `PyObject` have been cleaned up throughout the codebase and replaced with `PyRef`.

http://reviews.llvm.org/D13617

Files:
  source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13617.37018.patch
Type: text/x-patch
Size: 70656 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151010/7acd0ba4/attachment-0001.bin>