benkyd/tinyobjloader
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix of important memory leak in Python module

Browse Source 
Dear all,
As described in the issue https://github.com/syoyo/tinyobjloader/issues/188#issue-385341218 there is a memory leak in the function pyLoadObj(). The reference count for all created Python objects must be decreased after they are fed into the lists and dictionaries. The only  exception from this is the function PyTuple_SetItem() in pyTupleFromfloat3(), which decreases the reference counter of the Python object *tuple automatically by calling this function. In all other cases like PyList_Insert(), the references are only borrowed and not decreased. Therefore, each created Python object will remain in the heap, since there is one reference to it left in the counter.

These facts are explained in https://docs.python.org/2/c-api/intro.html#reference-counts in detail.

Best regards
Martin.
P.S. sorry, that i did not put that much effort into a more readable code and just inserted the Py_DECREF() function at every necessary position.
This commit is contained in:
Martin Thümmel
2018-11-28 17:16:12 +01:00
committed by GitHub
parent 59b4d7ccef
commit 178ef391c7
1 changed files with 77 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

148
python/main.cpp
View File

@@ -18,11 +18,9 @@ typedef std::vector<int> vecti;
PyObject* pyTupleFromfloat3(float array[3]) { PyObject* pyTupleFromfloat3(float array[3]) {
int i; int i;
PyObject* tuple = PyTuple_New(3); PyObject* tuple = PyTuple_New(3);
for (i = 0; i <= 2; i++) { for (i = 0; i <= 2; i++) {
PyTuple_SetItem(tuple, i, PyFloat_FromDouble(array[i])); PyTuple_SetItem(tuple, i, PyFloat_FromDouble(array[i]));
} }
return tuple; return tuple;
} }
@@ -30,32 +28,27 @@ extern "C" {
static PyObject* pyLoadObj(PyObject* self, PyObject* args) { static PyObject* pyLoadObj(PyObject* self, PyObject* args) {
PyObject *rtndict, *pyshapes, *pymaterials, *pymaterial_indices, *attribobj, *current, *meshobj; PyObject *rtndict, *pyshapes, *pymaterials, *pymaterial_indices, *attribobj, *current, *meshobj;
char const* current_name; char const* current_name;
char const* filename; char const* filename;
vectd vect; vectd vect;
std::vector<tinyobj::index_t> indices; std::vector<tinyobj::index_t> indices;
std::vector<unsigned char> face_verts; std::vector<unsigned char> face_verts;
tinyobj::attrib_t attrib; tinyobj::attrib_t attrib;
std::vector<tinyobj::shape_t> shapes; std::vector<tinyobj::shape_t> shapes;
std::vector<tinyobj::material_t> materials; std::vector<tinyobj::material_t> materials;
if (!PyArg_ParseTuple(args, "s", &filename)) return NULL; if (!PyArg_ParseTuple(args, "s", &filename)) return NULL;
std::string err; std::string err, warn;
tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename); tinyobj::LoadObj(&attrib, &shapes, &materials, &warn, &err, filename);
pyshapes = PyDict_New(); pyshapes = PyDict_New();
pymaterials = PyDict_New(); pymaterials = PyDict_New();
pymaterial_indices = PyList_New(0); pymaterial_indices = PyList_New(0);
rtndict = PyDict_New(); rtndict = PyDict_New();
attribobj = PyDict_New(); attribobj = PyDict_New();
for (int i = 0; i <= 2; i++) { for (int i = 0; i <= 2; i++) {
current = PyList_New(0); current = PyList_New(0);
switch (i) { switch (i) {
case 0: case 0:
current_name = "vertices"; current_name = "vertices";
@@ -72,124 +65,137 @@ static PyObject* pyLoadObj(PyObject* self, PyObject* args) {
} }
for (vectd::iterator it = vect.begin(); it != vect.end(); it++) { for (vectd::iterator it = vect.begin(); it != vect.end(); it++) {
PyList_Insert(current, it - vect.begin(), PyFloat_FromDouble(*it)); PyObject* value = PyFloat_FromDouble(*it);
PyList_Insert(current, it - vect.begin(), value);
Py_DECREF(value);
} }
PyDict_SetItemString(attribobj, current_name, current); PyDict_SetItemString(attribobj, current_name, current);
Py_DECREF(current);
} }
for (std::vector<tinyobj::shape_t>::iterator shape = shapes.begin(); for (std::vector<tinyobj::shape_t>::iterator shape = shapes.begin(); shape != shapes.end(); shape++) {
shape != shapes.end(); shape++) {
meshobj = PyDict_New(); meshobj = PyDict_New();
tinyobj::mesh_t cm = (*shape).mesh; tinyobj::mesh_t cm = (*shape).mesh;
{ {
current = PyList_New(0); current = PyList_New(0);
for (size_t i = 0; i < cm.indices.size(); i++) { for (size_t i = 0; i < cm.indices.size(); i++) {
// Flatten index array: v_idx, vn_idx, vt_idx, v_idx, vn_idx, vt_idx, // Flatten index array: v_idx, vn_idx, vt_idx, v_idx, vn_idx, vt_idx,
// ... PyObject* value = PyLong_FromLong(cm.indices[i].vertex_index);
PyList_Insert(current, 3 * i + 0, PyList_Insert(current, 3 * i + 0, value);
PyLong_FromLong(cm.indices[i].vertex_index)); Py_DECREF(value);
PyList_Insert(current, 3 * i + 1, value = PyLong_FromLong(cm.indices[i].normal_index);
PyLong_FromLong(cm.indices[i].normal_index)); PyList_Insert(current, 3 * i + 1, value);
PyList_Insert(current, 3 * i + 2, Py_DECREF(value);
PyLong_FromLong(cm.indices[i].texcoord_index)); value = PyLong_FromLong(cm.indices[i].texcoord_index);
PyList_Insert(current, 3 * i + 2, value);
Py_DECREF(value);
} }
PyDict_SetItemString(meshobj, "indices", current); PyDict_SetItemString(meshobj, "indices", current);
Py_DECREF(current);
} }
{ {
current = PyList_New(0); current = PyList_New(0);
for (size_t i = 0; i < cm.num_face_vertices.size(); i++) { for (size_t i = 0; i < cm.num_face_vertices.size(); i++) {
// Widen data type to long. // Widen data type to long.
PyList_Insert(current, i, PyLong_FromLong(cm.num_face_vertices[i])); PyObject* value = PyLong_FromLong(cm.num_face_vertices[i]);
PyList_Insert(current, i, value);
Py_DECREF(value);
} }
PyDict_SetItemString(meshobj, "num_face_vertices", current); PyDict_SetItemString(meshobj, "num_face_vertices", current);
Py_DECREF(current);
} }
{ {
current = PyList_New(0); current = PyList_New(0);
for (size_t i = 0; i < cm.material_ids.size(); i++) { for (size_t i = 0; i < cm.material_ids.size(); i++) {
PyList_Insert(current, i, PyLong_FromLong(cm.material_ids[i])); PyObject* value = PyLong_FromLong(cm.material_ids[i]);
PyList_Insert(current, i, value);
Py_DECREF(value);
} }
PyDict_SetItemString(meshobj, "material_ids", current); PyDict_SetItemString(meshobj, "material_ids", current);
Py_DECREF(current);
} }
PyDict_SetItemString(pyshapes, (*shape).name.c_str(), meshobj); PyDict_SetItemString(pyshapes, (*shape).name.c_str(), meshobj);
Py_DECREF(meshobj);
} }
for (std::vector<tinyobj::material_t>::iterator mat = materials.begin(); for (std::vector<tinyobj::material_t>::iterator mat = materials.begin(); mat != materials.end(); mat++) {
mat != materials.end(); mat++) {
PyObject* matobj = PyDict_New(); PyObject* matobj = PyDict_New();
PyObject* unknown_parameter = PyDict_New(); PyObject* unknown_parameter = PyDict_New();
for (std::map<std::string, std::string>::iterator p = for (std::map<std::string, std::string>::iterator p = mat->unknown_parameter.begin(); p != mat->unknown_parameter.end(); ++p) {
mat->unknown_parameter.begin(); PyObject* value = PyUnicode_FromString(p->second.c_str());
p != mat->unknown_parameter.end(); ++p) { PyDict_SetItemString(unknown_parameter, p->first.c_str(), value);
PyDict_SetItemString(unknown_parameter, p->first.c_str(), Py_DECREF(value);
PyUnicode_FromString(p->second.c_str()));
} }
PyDict_SetItemString(matobj, "shininess", PyObject* value = PyFloat_FromDouble(mat->shininess);
PyFloat_FromDouble(mat->shininess)); PyDict_SetItemString(matobj, "shininess", value);
PyDict_SetItemString(matobj, "ior", PyFloat_FromDouble(mat->ior)); Py_DECREF(value);
PyDict_SetItemString(matobj, "dissolve", value = PyFloat_FromDouble(mat->ior);
PyFloat_FromDouble(mat->dissolve)); PyDict_SetItemString(matobj, "ior", value);
PyDict_SetItemString(matobj, "illum", PyLong_FromLong(mat->illum)); Py_DECREF(value);
PyDict_SetItemString(matobj, "ambient_texname", value = PyFloat_FromDouble(mat->dissolve);
PyUnicode_FromString(mat->ambient_texname.c_str())); PyDict_SetItemString(matobj, "dissolve", value);
PyDict_SetItemString(matobj, "diffuse_texname", Py_DECREF(value);
PyUnicode_FromString(mat->diffuse_texname.c_str())); value = PyLong_FromLong(mat->illum);
PyDict_SetItemString(matobj, "specular_texname", PyDict_SetItemString(matobj, "illum", value);
PyUnicode_FromString(mat->specular_texname.c_str())); Py_DECREF(value);
PyDict_SetItemString( value = PyUnicode_FromString(mat->ambient_texname.c_str());
matobj, "specular_highlight_texname", PyDict_SetItemString(matobj, "ambient_texname", value);
PyUnicode_FromString(mat->specular_highlight_texname.c_str())); Py_DECREF(value);
PyDict_SetItemString(matobj, "bump_texname", value = PyUnicode_FromString(mat->diffuse_texname.c_str());
PyUnicode_FromString(mat->bump_texname.c_str())); PyDict_SetItemString(matobj, "diffuse_texname", value);
PyDict_SetItemString( Py_DECREF(value);
matobj, "displacement_texname", value = PyUnicode_FromString(mat->specular_texname.c_str());
PyUnicode_FromString(mat->displacement_texname.c_str())); PyDict_SetItemString(matobj, "specular_texname", value);
PyDict_SetItemString(matobj, "alpha_texname", Py_DECREF(value);
PyUnicode_FromString(mat->alpha_texname.c_str())); value = PyUnicode_FromString(mat->specular_highlight_texname.c_str());
PyDict_SetItemString(matobj, "specular_highlight_texname", value);
Py_DECREF(value);
value = PyUnicode_FromString(mat->bump_texname.c_str());
PyDict_SetItemString(matobj, "bump_texname", value);
Py_DECREF(value);
value = PyUnicode_FromString(mat->displacement_texname.c_str());
PyDict_SetItemString(matobj, "displacement_texname", value);
Py_DECREF(value);
value = PyUnicode_FromString(mat->alpha_texname.c_str());
PyDict_SetItemString(matobj, "alpha_texname", value);
Py_DECREF(value);
PyDict_SetItemString(matobj, "ambient", pyTupleFromfloat3(mat->ambient)); PyDict_SetItemString(matobj, "ambient", pyTupleFromfloat3(mat->ambient));
PyDict_SetItemString(matobj, "diffuse", pyTupleFromfloat3(mat->diffuse)); PyDict_SetItemString(matobj, "diffuse", pyTupleFromfloat3(mat->diffuse));
PyDict_SetItemString(matobj, "specular", PyDict_SetItemString(matobj, "specular", pyTupleFromfloat3(mat->specular));
pyTupleFromfloat3(mat->specular)); PyDict_SetItemString(matobj, "transmittance", pyTupleFromfloat3(mat->transmittance));
PyDict_SetItemString(matobj, "transmittance", PyDict_SetItemString(matobj, "emission", pyTupleFromfloat3(mat->emission));
pyTupleFromfloat3(mat->transmittance));
PyDict_SetItemString(matobj, "emission",
pyTupleFromfloat3(mat->emission));
PyDict_SetItemString(matobj, "unknown_parameter", unknown_parameter); PyDict_SetItemString(matobj, "unknown_parameter", unknown_parameter);
Py_DECREF(unknown_parameter);
PyDict_SetItemString(pymaterials, mat->name.c_str(), matobj); PyDict_SetItemString(pymaterials, mat->name.c_str(), matobj);
PyList_Append(pymaterial_indices, PyUnicode_FromString(mat->name.c_str())); Py_DECREF(matobj);
value = PyUnicode_FromString(mat->name.c_str());
PyList_Append(pymaterial_indices, value);
Py_DECREF(value);
} }
PyDict_SetItemString(rtndict, "shapes", pyshapes); PyDict_SetItemString(rtndict, "shapes", pyshapes);
Py_DECREF(pyshapes);
PyDict_SetItemString(rtndict, "materials", pymaterials); PyDict_SetItemString(rtndict, "materials", pymaterials);
Py_DECREF(pymaterials);
PyDict_SetItemString(rtndict, "material_indices", pymaterial_indices); PyDict_SetItemString(rtndict, "material_indices", pymaterial_indices);
Py_DECREF(pymaterial_indices);
PyDict_SetItemString(rtndict, "attribs", attribobj); PyDict_SetItemString(rtndict, "attribs", attribobj);
Py_DECREF(attribobj);
return rtndict; return rtndict;
} }
static PyMethodDef mMethods[] = { static PyMethodDef mMethods[] = {
{"LoadObj", pyLoadObj, METH_VARARGS}, {NULL, NULL, 0, NULL} {"LoadObj", pyLoadObj, METH_VARARGS}, {NULL, NULL, 0, NULL}
}; };
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "tinyobjloader", static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "tinyobjloader", NULL, -1, mMethods};
NULL, -1, mMethods};
PyMODINIT_FUNC PyInit_tinyobjloader(void) { PyMODINIT_FUNC PyInit_tinyobjloader(void) {
return PyModule_Create(&moduledef); return PyModule_Create(&moduledef);