[MOAB-dev] memory leak in AEntityFactory.cpp
Vijay S. Mahadevan
vijay.m at gmail.com
Tue May 19 22:10:30 CDT 2020
> That happens, when one add
>
> EntityHandle test_add;
> CHKERR get_moab().create_meshset(MESHSET_SET | MESHSET_TRACK_OWNER, test_add);
> CHKERR get_moab().add_entities(test_add, &meshset, 1);
>
> then adjacency is created between meshet. I hope that I understand that correctly. But I do not know how MOAB manages that case, when memory is released. Any Ideas, I am on good track?
If I understand correctly, you are saying that the memory leak is
triggered only if you use MESHSET_TRACK_OWNER when set is created ?
OTOH I do not know why that would be the case but we will have to
create a mini test to track that down. test/MBTest.cpp already has
several such tests and we do have valgrind builds on our nightly
checks. I will look more closely if something similar triggers there.
Best,
Vijay
On Tue, May 19, 2020 at 3:37 PM Lukasz Kaczmarczyk
<Lukasz.Kaczmarczyk at glasgow.ac.uk> wrote:
>
> Vijay,
>
> That happens, when one add
>
> EntityHandle test_add;
> CHKERR get_moab().create_meshset(MESHSET_SET | MESHSET_TRACK_OWNER, test_add);
> CHKERR get_moab().add_entities(test_add, &meshset, 1);
>
> then adjacency is created between meshet. I hope that I understand that correctly. But I do not know how MOAB manages that case, when memory is released. Any Ideas, I am on good track?
>
> This is strange case with MESHSET_TRACK_OWNER, anyway, I will remove this from the code.
>
> Kind regards,
> Lukasz
>
>
>
> On 18 May 2020, at 18:57, Lukasz Kaczmarczyk <Lukasz.Kaczmarczyk at glasgow.ac.uk> wrote:
>
> Vijay,
>
> No, it is something different. In the code below, all looks OK. I need some more time to understand this better.
>
>
> Regards,
> Lukasz
>
> On 18 May 2020, at 17:04, Lukasz Kaczmarczyk <Lukasz.Kaczmarczyk at glasgow.ac.uk<mailto:Lukasz.Kaczmarczyk at glasgow.ac.uk>> wrote:
>
> OK, I will do tiny PR.
>
> On 18 May 2020, at 16:31, Vijay S. Mahadevan <vijay.m at gmail.com<mailto:vijay.m at gmail.com><mailto:vijay.m at gmail.com>> wrote:
>
> Hi Lukasz,
>
> Thanks for raising this issue.
>
> In case of not the case (true == both_ways && to_type != MBVERTEX), or result != MB_SUCESS, should we realise adj_list_ptr. I can do PR, if needed.
>
> Yes, this is correct. We should release the memory in the else block.
> We would appreciate a PR so that this can be quickly reviewed and
> merged. Although a much cleaner option would be to actually not use
> the pointer allocation here for adj_list_ptr at all. I'll need to look
> at the code/call stack more closely to see if this would have any side
> effects.
>
> FYI, we are preparing for a new release by end of the week and so
> would be good to get this resolved soon.
>
> Thanks,
> Vijay
>
> On Mon, May 18, 2020 at 8:46 AM Lukasz Kaczmarczyk
> <Lukasz.Kaczmarczyk at glasgow.ac.uk<mailto:Lukasz.Kaczmarczyk at glasgow.ac.uk><mailto:Lukasz.Kaczmarczyk at glasgow.ac.uk>> wrote:
>
> Hello,
>
> I encounter following leak,
>
> ==2639== 40,680 bytes in 5,085 blocks are indirectly lost in loss record 62 of 63
> ==2639== at 0x4837DBF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==2639== by 0x4EA49CB: __gnu_cxx::new_allocator<unsigned long>::allocate(unsigned long, void const*) (new_allocator.h:111)
> ==2639== by 0x4E90C2B: std::allocator_traits<std::allocator<unsigned long> >::allocate(std::allocator<unsigned long>&, unsigned long) (alloc_traits.h:436)
> ==2639== by 0x4E75AB7: std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_M_allocate(unsigned long) (stl_vector.h:296)
> ==2639== by 0x4FF60A4: void std::vector<unsigned long, std::allocator<unsigned long> >::_M_realloc_insert<unsigned long const&>(__gnu_cxx::__normal_iterator<unsigned long*, std::vector<unsigned long, std::allocator<unsigned long> > >, unsigned long const&) (vector.tcc:427)
> ==2639== by 0x8652FCC: push_back (stl_vector.h:1085)
> ==2639== by 0x8652FCC: moab::AEntityFactory::add_adjacency(unsigned long, unsigned long, bool) (AEntityFactory.cpp:393)
> ==2639== by 0x8736ED5: ranged_insert_entities (MeshSet.cpp:613)
> ==2639== by 0x8736ED5: moab::MeshSet::insert_entity_ranges(moab::Range const&, unsigned long, moab::AEntityFactory*) (MeshSet.cpp:985)
> ==2639== by 0x89A2FDF: moab::ReadHDF5::read_set_data(moab::Range const&, unsigned long, moab::ReadHDF5Dataset&, moab::ReadHDF5::SetMode, moab::Range*) (ReadHDF5.cpp:2568)
> ==2639== by 0x89B33FF: moab::ReadHDF5::read_sets(moab::Range const&) (ReadHDF5.cpp:1958)
> ==2639== by 0x89B6FF9: moab::ReadHDF5::load_file_impl(moab::FileOptions const&) (ReadHDF5.cpp:647)
> ==2639== by 0x89B7662: moab::ReadHDF5::load_file(char const*, unsigned long const*, moab::FileOptions const&, moab::ReaderIface::SubsetList const*, moab::TagInfo* const*) (ReadHDF5.cpp:555)
> ==2639== by 0x86842B9: moab::Core::serial_load_file(char const*, unsigned long const*, moab::FileOptions const&, moab::ReaderIface::SubsetList const*, moab::TagInfo* const*) (Core.cpp:600)
> ==2639==
> ==2639== 162,720 (122,040 direct, 40,680 indirect) bytes in 5,085 blocks are definitely lost in loss record 63 of 63
> ==2639== at 0x4837DBF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==2639== by 0x8652639: moab::AEntityFactory::get_adjacencies(unsigned long, std::vector<unsigned long, std::allocator<unsigned long> >*&, bool) (AEntityFactory.cpp:565)
> ==2639== by 0x8652EB5: moab::AEntityFactory::add_adjacency(unsigned long, unsigned long, bool) (AEntityFactory.cpp:376)
> ==2639== by 0x8736ED5: ranged_insert_entities (MeshSet.cpp:613)
> ==2639== by 0x8736ED5: moab::MeshSet::insert_entity_ranges(moab::Range const&, unsigned long, moab::AEntityFactory*) (MeshSet.cpp:985)
> ==2639== by 0x89A2FDF: moab::ReadHDF5::read_set_data(moab::Range const&, unsigned long, moab::ReadHDF5Dataset&, moab::ReadHDF5::SetMode, moab::Range*) (ReadHDF5.cpp:2568)
> ==2639== by 0x89B33FF: moab::ReadHDF5::read_sets(moab::Range const&) (ReadHDF5.cpp:1958)
> ==2639== by 0x89B6FF9: moab::ReadHDF5::load_file_impl(moab::FileOptions const&) (ReadHDF5.cpp:647)
> ==2639== by 0x89B7662: moab::ReadHDF5::load_file(char const*, unsigned long const*, moab::FileOptions const&, moab::ReaderIface::SubsetList const*, moab::TagInfo* const*) (ReadHDF5.cpp:555)
> ==2639== by 0x86842B9: moab::Core::serial_load_file(char const*, unsigned long const*, moab::FileOptions const&, moab::ReaderIface::SubsetList const*, moab::TagInfo* const*) (Core.cpp:600)
> ==2639== by 0x8684E57: moab::Core::load_file(char const*, unsigned long const*, char const*, char const*, int const*, int) (Core.cpp:514)
> ==2639== by 0x4FB5E37: FractureMechanics::MWLSApprox::loadMWLSMesh(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (MWLS.cpp:100)
> ==2639== by 0x1A5FE4: main (mwls_at_gauss_point.cpp:166)
>
> In following function,
>
> ErrorCode AEntityFactory::add_adjacency(EntityHandle from_ent,
> EntityHandle to_ent,
> const bool both_ways)
> {
> EntityType to_type = TYPE_FROM_HANDLE(to_ent);
>
> if (to_type == MBVERTEX)
> return MB_ALREADY_ALLOCATED;
>
>
>
> AdjacencyVector *adj_list_ptr = NULL;
> ErrorCode result = get_adjacencies( from_ent, adj_list_ptr, true );
> if (MB_SUCCESS != result)
> return result;
>
> // get an iterator to the right spot in this sorted vector
> AdjacencyVector::iterator adj_iter;
> if (!adj_list_ptr->empty())
> {
> adj_iter = std::lower_bound(adj_list_ptr->begin(), adj_list_ptr->end(),
> to_ent);
>
> if ( adj_iter == adj_list_ptr->end() || to_ent != *adj_iter )
> {
> adj_list_ptr->insert(adj_iter, to_ent);
> }
> }
> else
> adj_list_ptr->push_back(to_ent);
>
> // if both_ways is true, recursively call this function
> if (true == both_ways && to_type != MBVERTEX)
> result = add_adjacency(to_ent, from_ent, false);
>
> return result;
> }
>
> In case of not the case (true == both_ways && to_type != MBVERTEX), or result != MB_SUCESS, should we realise adj_list_ptr. I can do PR, if needed.
>
>
> Kind regards,
> Lukasz
>
>
>
>
More information about the moab-dev
mailing list