Memory issue fix #57
Memory issue fix #57
Conversation
Freed up memories created by pointers in the function "destroy_geom_box_tree" to avoid memory leak.
Made the function "init_prism()" allocate new arrays only at the first time call.
| if (t->nobjects && t->objects) FREE(t->objects); | ||
| if (t->nobjects && t->objects) { | ||
| if (t->objects->o) | ||
| if (t->objects->o->which_subclass == GEOM PRISM) { |
There was a problem hiding this comment.
This doesn't make sense to me. t->objects is an array of pointers to objects, but it doesn't "own" the objects and is not responsible for freeing them. (Even if it were, this code is only freeing the first object in the list, and only for prisms?)
There was a problem hiding this comment.
Thanks for the feedback. If I understand correctly, the purpose of this function is to destroy (free) all the memory associated with a geom_box_tree_struct object. Since we do not have the C++ style destruction function, we need to manually free all the arrays from bottom to top. The chain I found is: geom_box_object --> geometric_object --> prism_struct --> vectors3_list --> items. If we only free the top level object, the lower level memory allocated by pointers will be leaked (since no pointers are cleaned, and not pointing to the heap arrays).
I think only prism structure has this issue, other geometry objects do not have because they do not have heap arrays allocated by pointers.
There was a problem hiding this comment.
The geom_box_tree object is used in situations like this or this or this where it is an auxiliary data structure to a linear geometric_object_list — the objects should only be freed when the corresponding list is destroyed (which should happen after the tree is destroyed). (Moreover, the same object may appear in multiple branches of the tree, so you have to be careful not to free it multiple times.)
There was a problem hiding this comment.
Thanks for the explanation. Since free of NULL pointer is safe, that is why NULL is assigned to the pointers after memory is freed to avoid double deletion.
There are no such memory leak issue in simulation (forward and adjoint) flow. It only happens in calculate_gradient. The flow is: calculate_gradient -> get_gradient -> destroy_geom_box_tree in "meep-python.cxx".
Also could you please point me to the place we destruct geometric_object_list since I did not find it? Thanks.
CONTEX: The same pointer for a prism-typed object are used to point to a newly allocated array so that the previously allocated arrays do not have any pointer to control, thus memory leaks happen. We would like to avoid repeated new memory allocations, and reuse the existing arrays.
SCOPE: