Skip to content

Commit 178ef39

Browse files
mathuemathue
authored
Fix of important memory leak in Python module
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.
1 parent 59b4d7c commit 178ef39

File tree

1 file changed

+77
-71
lines changed

1 file changed

+77
-71
lines changed

‎python/main.cpp‎

Lines changed: 77 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -18,44 +18,37 @@ typedef std::vector<int> vecti;
1818
PyObject* pyTupleFromfloat3(float array[3]) {
1919
int i;
2020
PyObject* tuple = PyTuple_New(3);
21-
2221
for (i = 0; i <= 2; i++) {
2322
PyTuple_SetItem(tuple, i, PyFloat_FromDouble(array[i]));
2423
}
25-
2624
return tuple;
2725
}
2826

2927
extern "C" {
3028

3129
static PyObject* pyLoadObj(PyObject* self, PyObject* args) {
3230
PyObject *rtndict, *pyshapes, *pymaterials, *pymaterial_indices, *attribobj, *current, *meshobj;
33-
3431
char const* current_name;
3532
char const* filename;
3633
vectd vect;
3734
std::vector<tinyobj::index_t> indices;
3835
std::vector<unsigned char> face_verts;
39-
4036
tinyobj::attrib_t attrib;
4137
std::vector<tinyobj::shape_t> shapes;
4238
std::vector<tinyobj::material_t> materials;
4339

4440
if (!PyArg_ParseTuple(args, "s", &filename)) return NULL;
4541

46-
std::string err;
47-
tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename);
48-
42+
std::string err, warn;
43+
tinyobj::LoadObj(&attrib, &shapes, &materials, &warn, &err, filename);
4944
pyshapes = PyDict_New();
5045
pymaterials = PyDict_New();
5146
pymaterial_indices = PyList_New(0);
5247
rtndict = PyDict_New();
53-
5448
attribobj = PyDict_New();
5549

5650
for (int i = 0; i <= 2; i++) {
5751
current = PyList_New(0);
58-
5952
switch (i) {
6053
case 0:
6154
current_name = "vertices";
@@ -72,124 +65,137 @@ static PyObject* pyLoadObj(PyObject* self, PyObject* args) {
7265
}
7366

7467
for (vectd::iterator it = vect.begin(); it != vect.end(); it++) {
75-
PyList_Insert(current, it - vect.begin(), PyFloat_FromDouble(*it));
68+
PyObject* value = PyFloat_FromDouble(*it);
69+
PyList_Insert(current, it - vect.begin(), value);
70+
Py_DECREF(value);
7671
}
77-
7872
PyDict_SetItemString(attribobj, current_name, current);
73+
Py_DECREF(current);
7974
}
8075

81-
for (std::vector<tinyobj::shape_t>::iterator shape = shapes.begin();
82-
shape != shapes.end(); shape++) {
76+
for (std::vector<tinyobj::shape_t>::iterator shape = shapes.begin(); shape != shapes.end(); shape++) {
8377
meshobj = PyDict_New();
8478
tinyobj::mesh_t cm = (*shape).mesh;
8579

8680
{
8781
current = PyList_New(0);
88-
8982
for (size_t i = 0; i < cm.indices.size(); i++) {
9083
// Flatten index array: v_idx, vn_idx, vt_idx, v_idx, vn_idx, vt_idx,
91-
// ...
92-
PyList_Insert(current, 3 * i + 0,
93-
PyLong_FromLong(cm.indices[i].vertex_index));
94-
PyList_Insert(current, 3 * i + 1,
95-
PyLong_FromLong(cm.indices[i].normal_index));
96-
PyList_Insert(current, 3 * i + 2,
97-
PyLong_FromLong(cm.indices[i].texcoord_index));
84+
PyObject* value = PyLong_FromLong(cm.indices[i].vertex_index);
85+
PyList_Insert(current, 3 * i + 0, value);
86+
Py_DECREF(value);
87+
value = PyLong_FromLong(cm.indices[i].normal_index);
88+
PyList_Insert(current, 3 * i + 1, value);
89+
Py_DECREF(value);
90+
value = PyLong_FromLong(cm.indices[i].texcoord_index);
91+
PyList_Insert(current, 3 * i + 2, value);
92+
Py_DECREF(value);
9893
}
99-
10094
PyDict_SetItemString(meshobj, "indices", current);
95+
Py_DECREF(current);
10196
}
10297

10398
{
10499
current = PyList_New(0);
105-
106100
for (size_t i = 0; i < cm.num_face_vertices.size(); i++) {
107101
// Widen data type to long.
108-
PyList_Insert(current, i, PyLong_FromLong(cm.num_face_vertices[i]));
102+
PyObject* value = PyLong_FromLong(cm.num_face_vertices[i]);
103+
PyList_Insert(current, i, value);
104+
Py_DECREF(value);
109105
}
110-
111106
PyDict_SetItemString(meshobj, "num_face_vertices", current);
107+
Py_DECREF(current);
112108
}
113109

114110
{
115111
current = PyList_New(0);
116-
117112
for (size_t i = 0; i < cm.material_ids.size(); i++) {
118-
PyList_Insert(current, i, PyLong_FromLong(cm.material_ids[i]));
113+
PyObject* value = PyLong_FromLong(cm.material_ids[i]);
114+
PyList_Insert(current, i, value);
115+
Py_DECREF(value);
119116
}
120-
121117
PyDict_SetItemString(meshobj, "material_ids", current);
118+
Py_DECREF(current);
122119
}
123-
124120
PyDict_SetItemString(pyshapes, (*shape).name.c_str(), meshobj);
121+
Py_DECREF(meshobj);
125122
}
126123

127-
for (std::vector<tinyobj::material_t>::iterator mat = materials.begin();
128-
mat != materials.end(); mat++) {
124+
for (std::vector<tinyobj::material_t>::iterator mat = materials.begin(); mat != materials.end(); mat++) {
129125
PyObject* matobj = PyDict_New();
130126
PyObject* unknown_parameter = PyDict_New();
131127

132-
for (std::map<std::string, std::string>::iterator p =
133-
mat->unknown_parameter.begin();
134-
p != mat->unknown_parameter.end(); ++p) {
135-
PyDict_SetItemString(unknown_parameter, p->first.c_str(),
136-
PyUnicode_FromString(p->second.c_str()));
128+
for (std::map<std::string, std::string>::iterator p = mat->unknown_parameter.begin(); p != mat->unknown_parameter.end(); ++p) {
129+
PyObject* value = PyUnicode_FromString(p->second.c_str());
130+
PyDict_SetItemString(unknown_parameter, p->first.c_str(), value);
131+
Py_DECREF(value);
137132
}
138133

139-
PyDict_SetItemString(matobj, "shininess",
140-
PyFloat_FromDouble(mat->shininess));
141-
PyDict_SetItemString(matobj, "ior", PyFloat_FromDouble(mat->ior));
142-
PyDict_SetItemString(matobj, "dissolve",
143-
PyFloat_FromDouble(mat->dissolve));
144-
PyDict_SetItemString(matobj, "illum", PyLong_FromLong(mat->illum));
145-
PyDict_SetItemString(matobj, "ambient_texname",
146-
PyUnicode_FromString(mat->ambient_texname.c_str()));
147-
PyDict_SetItemString(matobj, "diffuse_texname",
148-
PyUnicode_FromString(mat->diffuse_texname.c_str()));
149-
PyDict_SetItemString(matobj, "specular_texname",
150-
PyUnicode_FromString(mat->specular_texname.c_str()));
151-
PyDict_SetItemString(
152-
matobj, "specular_highlight_texname",
153-
PyUnicode_FromString(mat->specular_highlight_texname.c_str()));
154-
PyDict_SetItemString(matobj, "bump_texname",
155-
PyUnicode_FromString(mat->bump_texname.c_str()));
156-
PyDict_SetItemString(
157-
matobj, "displacement_texname",
158-
PyUnicode_FromString(mat->displacement_texname.c_str()));
159-
PyDict_SetItemString(matobj, "alpha_texname",
160-
PyUnicode_FromString(mat->alpha_texname.c_str()));
134+
PyObject* value = PyFloat_FromDouble(mat->shininess);
135+
PyDict_SetItemString(matobj, "shininess", value);
136+
Py_DECREF(value);
137+
value = PyFloat_FromDouble(mat->ior);
138+
PyDict_SetItemString(matobj, "ior", value);
139+
Py_DECREF(value);
140+
value = PyFloat_FromDouble(mat->dissolve);
141+
PyDict_SetItemString(matobj, "dissolve", value);
142+
Py_DECREF(value);
143+
value = PyLong_FromLong(mat->illum);
144+
PyDict_SetItemString(matobj, "illum", value);
145+
Py_DECREF(value);
146+
value = PyUnicode_FromString(mat->ambient_texname.c_str());
147+
PyDict_SetItemString(matobj, "ambient_texname", value);
148+
Py_DECREF(value);
149+
value = PyUnicode_FromString(mat->diffuse_texname.c_str());
150+
PyDict_SetItemString(matobj, "diffuse_texname", value);
151+
Py_DECREF(value);
152+
value = PyUnicode_FromString(mat->specular_texname.c_str());
153+
PyDict_SetItemString(matobj, "specular_texname", value);
154+
Py_DECREF(value);
155+
value = PyUnicode_FromString(mat->specular_highlight_texname.c_str());
156+
PyDict_SetItemString(matobj, "specular_highlight_texname", value);
157+
Py_DECREF(value);
158+
value = PyUnicode_FromString(mat->bump_texname.c_str());
159+
PyDict_SetItemString(matobj, "bump_texname", value);
160+
Py_DECREF(value);
161+
value = PyUnicode_FromString(mat->displacement_texname.c_str());
162+
PyDict_SetItemString(matobj, "displacement_texname", value);
163+
Py_DECREF(value);
164+
value = PyUnicode_FromString(mat->alpha_texname.c_str());
165+
PyDict_SetItemString(matobj, "alpha_texname", value);
166+
Py_DECREF(value);
161167
PyDict_SetItemString(matobj, "ambient", pyTupleFromfloat3(mat->ambient));
162168
PyDict_SetItemString(matobj, "diffuse", pyTupleFromfloat3(mat->diffuse));
163-
PyDict_SetItemString(matobj, "specular",
164-
pyTupleFromfloat3(mat->specular));
165-
PyDict_SetItemString(matobj, "transmittance",
166-
pyTupleFromfloat3(mat->transmittance));
167-
PyDict_SetItemString(matobj, "emission",
168-
pyTupleFromfloat3(mat->emission));
169+
PyDict_SetItemString(matobj, "specular", pyTupleFromfloat3(mat->specular));
170+
PyDict_SetItemString(matobj, "transmittance", pyTupleFromfloat3(mat->transmittance));
171+
PyDict_SetItemString(matobj, "emission", pyTupleFromfloat3(mat->emission));
169172
PyDict_SetItemString(matobj, "unknown_parameter", unknown_parameter);
170-
173+
Py_DECREF(unknown_parameter);
171174
PyDict_SetItemString(pymaterials, mat->name.c_str(), matobj);
172-
PyList_Append(pymaterial_indices, PyUnicode_FromString(mat->name.c_str()));
175+
Py_DECREF(matobj);
176+
value = PyUnicode_FromString(mat->name.c_str());
177+
PyList_Append(pymaterial_indices, value);
178+
Py_DECREF(value);
173179
}
174180

175181
PyDict_SetItemString(rtndict, "shapes", pyshapes);
182+
Py_DECREF(pyshapes);
176183
PyDict_SetItemString(rtndict, "materials", pymaterials);
184+
Py_DECREF(pymaterials);
177185
PyDict_SetItemString(rtndict, "material_indices", pymaterial_indices);
186+
Py_DECREF(pymaterial_indices);
178187
PyDict_SetItemString(rtndict, "attribs", attribobj);
179-
188+
Py_DECREF(attribobj);
180189
return rtndict;
181190
}
182191

183192
static PyMethodDef mMethods[] = {
184-
185193
{"LoadObj", pyLoadObj, METH_VARARGS}, {NULL, NULL, 0, NULL}
186-
187194
};
188195

189196
#if PY_MAJOR_VERSION >= 3
190197

191-
static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "tinyobjloader",
192-
NULL, -1, mMethods};
198+
static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "tinyobjloader", NULL, -1, mMethods};
193199

194200
PyMODINIT_FUNC PyInit_tinyobjloader(void) {
195201
return PyModule_Create(&moduledef);

0 commit comments

Comments
 (0)