fix two memory leaks in geom_epsilon class

[oskooi]

This PR fixes a couple of memory leaks in the geom_epsilon class. The first is a simple check to avoid duplicating the restricted_tree object in geom_epsilon::set_volume. The second is manually deleting the global variable default_material (which is a pointer to material_data) by calling a new function meep_geom::unset_default_material() at the end of two C++ unit tests (array-slice-ll.cpp and cyl-ellipsoid-ll.cpp).

With these two fixes, the address sanitizer in the GitHub Actions CI passes and can thus be enabled by default for all subsequent PRs.

[oskooi]

As an explanation for why the global variable default_material needed to be freed, here is the output from the address sanitizer when the line meep_geom::unset_default_material(); is not included at the end of array-slice-ll.cpp:

 HEAP SUMMARY:
     in use at exit: 1,136 bytes in 1 blocks
   total heap usage: 59,789 allocs, 59,788 frees, 12,636,705 bytes allocated

 1,136 bytes in 1 blocks are still reachable in loss record 1 of 1
    at 0x4839F2F: operator new(unsigned long) (vg_replace_malloc.c:422)
    by 0x49BD9DE: meep_geom::set_default_material(meep_geom::material_data*) (meepgeom.cpp:40)
    by 0x49C7AAA: meep_geom::make_geom_epsilon(meep::structure*, geometric_object_list*, vector3, bool, meep_geom::material_data*, meep_geom::material_type_\
list) (meepgeom.cpp:1939)
    by 0x49C80B7: meep_geom::set_materials_from_geometry(meep::structure*, geometric_object_list, vector3, bool, double, int, bool, meep_geom::material_data\
*, std::vector<meep_geom::absorber, std::allocator<meep_geom::absorber> >*, meep_geom::material_type_list) (meepgeom.cpp:1990)
    by 0x10D159: main (array-slice-ll.cpp:165)

 LEAK SUMMARY:
    definitely lost: 0 bytes in 0 blocks
    indirectly lost: 0 bytes in 0 blocks
      possibly lost: 0 bytes in 0 blocks
    still reachable: 1,136 bytes in 1 blocks
         suppressed: 0 bytes in 0 blocks

 ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The offending new occurs on line 40 of meepgeom.cpp in this block of the function set_default_material:

meep/src/meepgeom.cpp

Lines 39 to 43 in efe3b4b

	if (_default_material != NULL) {
	material_type new_material = new material_data();
	new_material->copy_from(*_default_material);
	default_material = (void *)new_material;
	}

This is why every time a geom_epsilon object is created, it calls set_default_material with the default argument vacuum (as in the C++ unit tests array-slice-ll.cpp and others) which results in a heap allocation.

[ahoenselaar]

Does this actually cause a test failure? The error summary indicates “0 errors”.

…

On Tue, Jan 4, 2022 at 9:38 PM Ardavan Oskooi ***@***.***> wrote: As an explanation for why the global variable default_material needed to be freed, here is the output from the address sanitizer when the line meep_geom::unset_default_material(); is *not* included at the end of array-slice-ll.cpp: HEAP SUMMARY: in use at exit: 1,136 bytes in 1 blocks total heap usage: 59,789 allocs, 59,788 frees, 12,636,705 bytes allocated 1,136 bytes in 1 blocks are still reachable in loss record 1 of 1 at 0x4839F2F: operator new(unsigned long) (vg_replace_malloc.c:422) by 0x49BD9DE: meep_geom::set_default_material(meep_geom::material_data*) (meepgeom.cpp:40) by 0x49C7AAA: meep_geom::make_geom_epsilon(meep::structure*, geometric_object_list*, vector3, bool, meep_geom::material_data*, meep_geom::material_type_\ list) (meepgeom.cpp:1939) by 0x49C80B7: meep_geom::set_materials_from_geometry(meep::structure*, geometric_object_list, vector3, bool, double, int, bool, meep_geom::material_data\ *, std::vector<meep_geom::absorber, std::allocator<meep_geom::absorber> >*, meep_geom::material_type_list) (meepgeom.cpp:1990) by 0x10D159: main (array-slice-ll.cpp:165) LEAK SUMMARY: definitely lost: 0 bytes in 0 blocks indirectly lost: 0 bytes in 0 blocks possibly lost: 0 bytes in 0 blocks still reachable: 1,136 bytes in 1 blocks suppressed: 0 bytes in 0 blocks ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) The offending new occurs on line 40 of meepgeom.cpp in this block of the function set_default_material: https://github.com/NanoComp/meep/blob/efe3b4b1fff538cf1b1ad2e34bb47c7a3ddfb28a/src/meepgeom.cpp#L39-L43 This is why every time a geom_epsilon object is created, it calls set_default_material with the default argument vacuum (as in the C++ unit tests array-slice-ll.cpp and others) which results in a heap allocation. — Reply to this email directly, view it on GitHub <#1877 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4VK52K2SZEA44H6ZKSHPDUUPKMZANCNFSM5LD6KX7Q> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[oskooi]

Does this actually cause a test failure? The error summary indicates “0 errors”.

Even though it is not listed as an error in the log output, it was causing the sanitizer on GitHub Actions to fail.