search

bertcarnell / lhs

Provides a number of methods for creating and augmenting Latin Hypercube Samples and Orthogonal Array Latin Hypercube Samples
https://bertcarnell.github.io/lhs/
GNU General Public License v3.0
44 stars 8 forks source link

Memory leak in bosebushl tests #28

Closed bertcarnell closed 4 years ago

bertcarnell commented 4 years ago

Memory leak not found in R CMD check --use-valgrind.

https://cran.r-project.org/web/checks/check_results_lhs.html

https://www.stats.ox.ac.uk/pub/bdr/memtests/valgrind/lhs/tests/testthat.Rout

==2401262== Memcheck, a memory error detector
==2401262== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2401262== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==2401262== Command: /data/blackswan/ripley/R/R-devel-vg/bin/exec/R -f testthat.R --restore --save --no-readline --vanilla
==2401262== 

R Under development (unstable) (2020-09-28 r79268) -- "Unsuffered Consequences"
Copyright (C) 2020 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(testthat)
> library(lhs)
> 
> test_check("lhs")
==2401262== Invalid read of size 4
==2401262==    at 0x173CA725: oacpp::oaconstruct::bosebushl(oacpp::GF&, int, bclib::matrix<int>&, int) (packages/tests-vg/lhs/src/construct.cpp:422)
==2401262==    by 0x173C3A59: oacpp::COrthogonalArray::bosebushl(int, int, int, int*) (packages/tests-vg/lhs/src/COrthogonalArray.cpp:174)
==2401262==    by 0x173E7889: oa_type2 (packages/tests-vg/lhs/src/oa_r.cpp:158)
==2401262==    by 0x49CD87: R_doDotCall (svn/R-devel/src/main/dotcode.c:610)
==2401262==    by 0x49D2A3: do_dotcall (svn/R-devel/src/main/dotcode.c:1281)
==2401262==    by 0x4D2BE6: bcEval (svn/R-devel/src/main/eval.c:7121)
==2401262==    by 0x4EF6F7: Rf_eval (svn/R-devel/src/main/eval.c:727)
==2401262==    by 0x4F110D: R_execClosure (svn/R-devel/src/main/eval.c:1895)
==2401262==    by 0x4F1E03: Rf_applyClosure (svn/R-devel/src/main/eval.c:1821)
==2401262==    by 0x4EF8C3: Rf_eval (svn/R-devel/src/main/eval.c:850)
==2401262==    by 0x4F3B99: do_set (svn/R-devel/src/main/eval.c:2967)
==2401262==    by 0x4EFB44: Rf_eval (svn/R-devel/src/main/eval.c:802)
==2401262==  Address 0x142c5100 is 0 bytes after a block of size 128 alloc'd
==2401262==    at 0x483AE7D: operator new(unsigned long) (/builddir/build/BUILD/valgrind-3.16.1/coregrind/m_replacemalloc/vg_replace_malloc.c:342)
==2401262==    by 0x173C4299: allocate (/usr/include/c++/10/ext/new_allocator.h:115)
==2401262==    by 0x173C4299: allocate (/usr/include/c++/10/bits/alloc_traits.h:460)
==2401262==    by 0x173C4299: _M_allocate (/usr/include/c++/10/bits/stl_vector.h:346)
==2401262==    by 0x173C4299: _M_create_storage (/usr/include/c++/10/bits/stl_vector.h:361)
==2401262==    by 0x173C4299: _Vector_base (/usr/include/c++/10/bits/stl_vector.h:305)
==2401262==    by 0x173C4299: vector (/usr/include/c++/10/bits/stl_vector.h:511)
==2401262==    by 0x173C4299: bclib::matrix<int>::matrix(unsigned long, unsigned long) (packages/tests-vg/lhs/src/matrix.h:488)
==2401262==    by 0x173CA501: oacpp::oaconstruct::bosebushl(oacpp::GF&, int, bclib::matrix<int>&, int) (packages/tests-vg/lhs/src/construct.cpp:401)
==2401262==    by 0x173C3A59: oacpp::COrthogonalArray::bosebushl(int, int, int, int*) (packages/tests-vg/lhs/src/COrthogonalArray.cpp:174)
==2401262==    by 0x173E7889: oa_type2 (packages/tests-vg/lhs/src/oa_r.cpp:158)
==2401262==    by 0x49CD87: R_doDotCall (svn/R-devel/src/main/dotcode.c:610)
==2401262==    by 0x49D2A3: do_dotcall (svn/R-devel/src/main/dotcode.c:1281)
==2401262==    by 0x4D2BE6: bcEval (svn/R-devel/src/main/eval.c:7121)
==2401262==    by 0x4EF6F7: Rf_eval (svn/R-devel/src/main/eval.c:727)
==2401262==    by 0x4F110D: R_execClosure (svn/R-devel/src/main/eval.c:1895)
==2401262==    by 0x4F1E03: Rf_applyClosure (svn/R-devel/src/main/eval.c:1821)
==2401262==    by 0x4EF8C3: Rf_eval (svn/R-devel/src/main/eval.c:850)
==2401262== 
══ testthat results  ═══════════════════════════════════════════════════════════
[ OK: 395 | SKIPPED: 0 | WARNINGS: 0 | FAILED: 0 ]
> 
> proc.time()
   user  system elapsed 
360.900   0.730 361.977 
==2401262== 
==2401262== HEAP SUMMARY:
==2401262==     in use at exit: 99,981,672 bytes in 19,556 blocks
==2401262==   total heap usage: 322,741 allocs, 303,185 frees, 364,624,316 bytes allocated
==2401262== 
==2401262== 11 bytes in 11 blocks are definitely lost in loss record 11 of 2,261
==2401262==    at 0x483AE7D: operator new(unsigned long) (/builddir/build/BUILD/valgrind-3.16.1/coregrind/m_replacemalloc/vg_replace_malloc.c:342)
==2401262==    by 0x173D5CD3: geneticLHS_cpp (packages/tests-vg/lhs/src/lhs_r.cpp:253)
==2401262==    by 0x49CD21: R_doDotCall (svn/R-devel/src/main/dotcode.c:624)
==2401262==    by 0x49D2A3: do_dotcall (svn/R-devel/src/main/dotcode.c:1281)
==2401262==    by 0x4D2BE6: bcEval (svn/R-devel/src/main/eval.c:7121)
==2401262==    by 0x4EF6F7: Rf_eval (svn/R-devel/src/main/eval.c:727)
==2401262==    by 0x4F110D: R_execClosure (svn/R-devel/src/main/eval.c:1895)
==2401262==    by 0x4F1E03: Rf_applyClosure (svn/R-devel/src/main/eval.c:1821)
==2401262==    by 0x4EF8C3: Rf_eval (svn/R-devel/src/main/eval.c:850)
==2401262==    by 0x49CDDB: R_doDotCall (svn/R-devel/src/main/dotcode.c:601)
==2401262==    by 0x4D99ED: bcEval (svn/R-devel/src/main/eval.c:7677)
==2401262==    by 0x4EF6F7: Rf_eval (svn/R-devel/src/main/eval.c:727)
==2401262== 
==2401262== LEAK SUMMARY:
==2401262==    definitely lost: 11 bytes in 11 blocks
==2401262==    indirectly lost: 0 bytes in 0 blocks
==2401262==      possibly lost: 0 bytes in 0 blocks
==2401262==    still reachable: 99,981,661 bytes in 19,545 blocks
==2401262==         suppressed: 0 bytes in 0 blocks
==2401262== Reachable blocks (those to which a pointer was found) are not shown.
==2401262== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2401262== 
==2401262== For lists of detected and suppressed errors, rerun with: -s
==2401262== ERROR SUMMARY: 9 errors from 2 contexts (suppressed: 0 from 0)
babaoppa commented 4 years ago

two issues described in this valgrind trace 1) Invalid read of size 4 in oacpp::oaconstruct::bosebushl(oacpp::GF&, int, bclib::matrix&, int) (packages/tests-vg/lhs/src/construct.cpp:422) 2) a memory leak 11 bytes in 11 blocks are definitely lost in loss record 11 of 2,261 ==2401262== at 0x483AE7D: operator new(unsigned long) (/builddir/build/BUILD/valgrind-3.16.1/coregrind/m_replacemalloc/vg_replace_malloc.c:342) ==2401262== by 0x173D5CD3: geneticLHS_cpp (packages/tests-vg/lhs/src/lhs_r.cpp:253)

bertcarnell commented 4 years ago

I have isolated the invalid read in boasebushl https://github.com/bertcarnell/lhs/commit/6c049248e9fdf49ab35970e37f63d5cc70303155

I also saw a way for the RNG seed pointer to be lost if the geneticLHS threw an exception. I added try-catch blocks to free the memory: https://github.com/bertcarnell/lhs/commit/1a5152e4e9cfc14d927fb712e9d314ff002d8f5f

There still seems to be a valgrind problem remaining, however.

bertcarnell commented 4 years ago

Fixed with this commit: https://github.com/bertcarnell/lhs/commit/3e9d73ae11d6078874073cc802e6dfe5b3866d70