dthuerck / mapmap_cpu

A high-performance general-purpose MRF MAP solver, heavily exploiting SIMD instructions.
BSD 3-Clause "New" or "Revised" License
102 stars 51 forks source link

Possible memory leak in pairwise_table.impl.h #9

Closed h00shi closed 6 years ago

h00shi commented 6 years ago

Hi,

In my application I have been running your solver multiple times in a single run, and I noticed a large memory build up. I also use the PairwiseTable object for assigning binary costs. Running the code in valgrind, I got the following warning.

==9218== Mismatched free() / delete / delete []
==9218==    at 0x4C2F24B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9218==    by 0x8FEC2F: std::default_delete<double>::operator()(double*) const (unique_ptr.h:76)
==9218==    by 0x8F7428: std::unique_ptr<double, std::default_delete<double> >::~unique_ptr() (unique_ptr.h:236)
==9218==    by 0x8EE71F: ~PairwiseTable (pairwise_table.impl.h:81)
==9218==    by 0x8EE71F: mapmap::PairwiseTable<double, 1u>::~PairwiseTable() (pairwise_table.impl.h:81)
==9218==    by 0x915285: std::default_delete<mapmap::PairwiseCosts<double, 1u> >::operator()(mapmap::PairwiseCosts<double, 1u>*) const (unique_ptr.h:76)
==9218==    by 0x90D4E8: std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >::~unique_ptr() (unique_ptr.h:236)
==9218==    by 0x91BFBA: void std::_Destroy<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > > >(std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*) (stl_construct.h:93)
==9218==    by 0x9169AE: void std::_Destroy_aux<false>::__destroy<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*>(std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*, std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*) (stl_construct.h:103)
==9218==    by 0x90FAEB: void std::_Destroy<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*>(std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*, std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*) (stl_construct.h:126)
==9218==    by 0x9097D6: void std::_Destroy<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*, std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > > >(std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*, std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >*, std::allocator<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > > >&) (stl_construct.h:151)
==9218==    by 0x8FF784: std::vector<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > >, std::allocator<std::unique_ptr<mapmap::PairwiseCosts<double, 1u>, std::default_delete<mapmap::PairwiseCosts<double, 1u> > > > >::~vector() (stl_vector.h:424)
==9218==    by 0x8F792F: ~Multilevel (multilevel.impl.h:99)
==9218==    by 0x8F792F: std::default_delete<mapmap::Multilevel<double, 1u> >::operator()(mapmap::Multilevel<double, 1u>*) const (unique_ptr.h:76)

==9218==  Address 0x88ff640 is 0 bytes inside a block of size 288 alloc'd
==9218==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9218==    by 0x90539F: PairwiseTable (pairwise_table.impl.h:33)
==9218==    by 0x90539F: mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}::operator()(tbb::blocked_range<unsigned long> const&) const (multilevel.impl.h:898)
==9218==    by 0x9479A6: tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>::run_body(tbb::blocked_range<unsigned long>&) (parallel_for.h:102)
==9218==    by 0x94187B: void tbb::interface9::internal::balancing_partition_type<tbb::interface9::internal::adaptive_mode<tbb::interface9::internal::auto_partition_type> >::work_balance<tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>, tbb::blocked_range<unsigned long> >(tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>&, tbb::blocked_range<unsigned long>&) (partitioner.h:446)
==9218==    by 0x9384B7: void tbb::interface9::internal::partition_type_base<tbb::interface9::internal::auto_partition_type>::execute<tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>, tbb::blocked_range<unsigned long> >(tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>&, tbb::blocked_range<unsigned long>&) (partitioner.h:257)
==9218==    by 0x92C8D7: tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>::execute() (parallel_for.h:127)
==9218==    by 0x4E60FDC: tbb::internal::custom_scheduler<tbb::internal::IntelSchedulerTraits>::local_wait_for_all(tbb::task&, tbb::task*) (in /usr/lib/x86_64-linux-gnu/libtbb.so.2)
==9218==    by 0x4E5E1EF: tbb::internal::generic_scheduler::local_spawn_root_and_wait(tbb::task&, tbb::task*&) (in /usr/lib/x86_64-linux-gnu/libtbb.so.2)
==9218==    by 0x8DFBFA: tbb::task::spawn_root_and_wait(tbb::task&) (task.h:736)
==9218==    by 0x9153E4: tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}, tbb::auto_partitioner const>::run(tbb::blocked_range<unsigned long> const&, {lambda(tbb::blocked_range<unsigned long> const&)#1} const&, tbb::auto_partitioner&) (parallel_for.h:90)
==9218==    by 0x90D79E: void tbb::parallel_for<tbb::blocked_range<unsigned long>, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1}>(tbb::blocked_range<unsigned long> const&, mapmap::Multilevel<double, 1u>::compute_level_pairwise()::{lambda(tbb::blocked_range<unsigned long> const&)#1} const&) (parallel_for.h:186)
==9218==    by 0x8D48D3: compute_level_pairwise (multilevel.impl.h:860)
==9218==    by 0x8D48D3: next_level (multilevel.impl.h:214)
==9218==    by 0x8D48D3: opt_step_multilevel (mapmap.impl.h:645)
==9218==    by 0x8D48D3: optimize (mapmap.impl.h:359)
==9218==    by 0x8D48D3: DGP::mrf::MRF_solver_mapmap::solve(std::vector<int, std::allocator<int> >&, double&) (mrf_solvers.cxx:496)

My guess is that the issue is the following. It seems that in pairwisetable.impl.h in line 34 a dynamic array is created via new[], which needs to be deleted via delete[]. However, the resutls are saved in a std::unique_ptr:

m_packed_table_storage = std::unique_ptr<_s_t<COSTTYPE, SIMDWIDTH>>(new
        _s_t<COSTTYPE, SIMDWIDTH>[padded_size]);

std::unique_ptr, on the other hand, will call delete rather than delete[], which causes this memory leak.

dthuerck commented 6 years ago

Hi,

thanks for opening the issue! I have fixed this and, in the process, found another bug in pairwise_table. Expect a commit for this and #10 soon.