valgrind memory leak from parallelFor

[mpadge]

Your example for parallelFor -- and indeed any use of parallelFor - generates a valgrind memory leak. Can be reproduced by pasting your suggested code in test.cpp, with the following three necessary lines pre-pended:

#include <algorithm> // std::transform

#include <Rcpp.h>
using namespace Rcpp;

Running valgrind on a file test.R containing just

Rcpp::sourceCpp ("test.cpp")
x <- parallelMatrixSqrt (matrix (1:100, nrow = 10))

via

R -d "valgrind --tool=memcheck --leak-check=full" -f test.R

generates the following output

==3116==
==3116== HEAP SUMMARY:
==3116==     in use at exit: 53,081,616 bytes in 10,368 blocks
==3116==   total heap usage: 35,858 allocs, 25,490 frees, 93,680,163 bytes allocated
==3116==
==3116== 640 bytes in 2 blocks are possibly lost in loss record 163 of 1,639
==3116==    at 0x4839B65: calloc (vg_replace_malloc.c:752)
==3116==    by 0x40128D2: allocate_dtv (in /usr/lib/ld-2.28.so)
==3116==    by 0x4013281: _dl_allocate_tls (in /usr/lib/ld-2.28.so)
==3116==    by 0x4D08698: pthread_create@@GLIBC_2.2.5 (in /usr/lib/libpthread-2.28.so)
==3116==    by 0xE8F8D64: launch (thread_monitor.h:221)
==3116==    by 0xE8F8D64: wake_or_launch (private_server.cpp:300)
==3116==    by 0xE8F8D64: tbb::internal::rml::private_server::wake_some(int) (private_server.cpp:394)
==3116==    by 0xE90144A: tbb::internal::generic_scheduler::local_spawn(tbb::task*, tbb::task*&) (scheduler.cpp:648)
==3116==    by 0xC77F37E: tbb::interface9::internal::start_for<tbb::blocked_range<unsigned long>, RcppParallel::(anonymous
namespace)::TBBWorker, tbb::auto_partitioner const>::execute() (in
/tmp/Rtmp4ECf2u/sourceCpp-x86_64-pc-linux-gnu-0.12.19/sourcecpp_c2c5511ea5e/sourceCpp_2.so)
==3116==    by 0xE90473C: tbb::internal::custom_scheduler<tbb::internal::IntelSchedulerTraits>::local_wait_for_all(tbb::task&, tbb::task*)
(custom_scheduler.h:509)
==3116==    by 0xE901BF8: tbb::internal::generic_scheduler::local_spawn_root_and_wait(tbb::task*, tbb::task*&) (scheduler.cpp:670)
==3116==    by 0xC780037: RcppParallel::tbbParallelFor(unsigned long, unsigned long, RcppParallel::Worker&, unsigned long) (in
/tmp/Rtmp4ECf2u/sourceCpp-x86_64-pc-linux-gnu-0.12.19/sourcecpp_c2c5511ea5e/sourceCpp_2.so)
==3116==    by 0xC77EC2B: parallelMatrixSqrt(Rcpp::Matrix<14, Rcpp::PreserveStorage>) (in
/tmp/Rtmp4ECf2u/sourceCpp-x86_64-pc-linux-gnu-0.12.19/sourcecpp_c2c5511ea5e/sourceCpp_2.so)
==3116==    by 0xC77EE08: sourceCpp_1_parallelMatrixSqrt (in
/tmp/Rtmp4ECf2u/sourceCpp-x86_64-pc-linux-gnu-0.12.19/sourcecpp_c2c5511ea5e/sourceCpp_2.so)
==3116==
==3116== 1,600 bytes in 5 blocks are possibly lost in loss record 213 of 1,639
==3116==    at 0x4839B65: calloc (vg_replace_malloc.c:752)
==3116==    by 0x40128D2: allocate_dtv (in /usr/lib/ld-2.28.so)
==3116==    by 0x4013281: _dl_allocate_tls (in /usr/lib/ld-2.28.so)
==3116==    by 0x4D08698: pthread_create@@GLIBC_2.2.5 (in /usr/lib/libpthread-2.28.so)
==3116==    by 0xE8F8D64: launch (thread_monitor.h:221)
==3116==    by 0xE8F8D64: wake_or_launch (private_server.cpp:300)
==3116==    by 0xE8F8D64: tbb::internal::rml::private_server::wake_some(int) (private_server.cpp:394)
==3116==    by 0xE8F9087: propagate_chain_reaction (private_server.cpp:161)
==3116==    by 0xE8F9087: tbb::internal::rml::private_worker::run() (private_server.cpp:261)
==3116==    by 0xE8F9109: tbb::internal::rml::private_worker::thread_routine(void*) (private_server.cpp:223)
==3116==    by 0x4D07A9C: start_thread (in /usr/lib/libpthread-2.28.so)
==3116==    by 0x4E1CA42: clone (in /usr/lib/libc-2.28.so)
==3116==
==3116== LEAK SUMMARY:
==3116==    definitely lost: 0 bytes in 0 blocks
==3116==    indirectly lost: 0 bytes in 0 blocks
==3116==      possibly lost: 2,240 bytes in 7 blocks
==3116==    still reachable: 53,079,376 bytes in 10,361 blocks
==3116==                       of which reachable via heuristic:
==3116==                         newarray           : 7,360 bytes in 4 blocks
==3116==         suppressed: 0 bytes in 0 blocks
==3116== Reachable blocks (those to which a pointer was found) are not shown.
==3116== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==3116==
==3116== For counts of detected and suppressed errors, rerun with: -v
==3116== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

A fix would be greatly appreciated!

> sessionInfo()
R version 3.5.1 (2018-07-02)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Arch Linux

Matrix products: default
BLAS: /usr/lib/libblas.so.3.8.0
LAPACK: /usr/lib/liblapack.so.3.8.0

locale:
 [1] LC_CTYPE=en_GB.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_GB.UTF-8        LC_COLLATE=en_GB.UTF-8
 [5] LC_MONETARY=en_GB.UTF-8    LC_MESSAGES=en_GB.UTF-8
 [7] LC_PAPER=en_GB.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
 [11] LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

loaded via a namespace (and not attached):
[1] compiler_3.5.1 tools_3.5.1    colorout_1.2-0 curl_3.2

[eddelbuettel]

It's been a moment since I mud-wrestled with valgrind but IIRC then

==3116== LEAK SUMMARY:
==3116==    definitely lost: 0 bytes in 0 blocks
==3116==    indirectly lost: 0 bytes in 0 blocks
==3116==      possibly lost: 2,240 bytes in 7 blocks
==3116==    still reachable: 53,079,376 bytes in 10,361 blocks

may be a nothing burger as we always get 'still reachable' on a dynamic language such as R.

FWIW I get something very similar on a basic Rcpp example of just returning twice an input vector:

==332827== HEAP SUMMARY:
==332827==     in use at exit: 53,512,483 bytes in 10,282 blocks
==332827==   total heap usage: 27,616 allocs, 17,334 frees, 84,878,116 bytes allocated
==332827== 
==332827== LEAK SUMMARY:
==332827==    definitely lost: 0 bytes in 0 blocks
==332827==    indirectly lost: 0 bytes in 0 blocks
==332827==      possibly lost: 0 bytes in 0 blocks
==332827==    still reachable: 53,512,483 bytes in 10,282 blocks
==332827==         suppressed: 0 bytes in 0 blocks
==332827== Reachable blocks (those to which a pointer was found) are not shown.
==332827== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==332827== 

So not sure yet there is something here. (It is sometimes insightful to construct "the other side" -- ie do an explicit malloc(), say, and then exit and see how that gets reported, likely as a "definitely lost".)

Edit: I did miss the possibly lost: 2,240 bytes on a first pass. Not sure that is meaningful though.

[kevinushey]

Also worth saying that valgrind is reporting the leak as occurring within libpthread through code called by TBB, so even if this is a real leak I have no idea how we could fix it without diving into TBB sources and that's always a quagmire.

[mpadge]

It seems to arise only from the allocation of a thread, because much larger tasks still only generate a couple thousand possibly lost bytes, so there's nothing too serious going on. It is nevertheless likely to be flagged by CRAN checks, which definitely object to possibly lost bytes, so at least this issue will provide a reference point for that.

[eddelbuettel]

Good follow-up trying to assess the implication. So yes, on balance, it seems to be "one of those things" we just have to live with. Shall we close this as a "non-action" item; Google will still find it.

[mpadge]

More info on this issue, which now prevents me from doing package checks with rocker/r-devel-ubsan-clang owing to failed R CMD build with an RcppParalllel dependency with this output:

=================================================================
==20456==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 3096 byte(s) in 3 object(s) allocated from:
    #0 0x4f5512 in operator new[](unsigned long) (/usr/local/lib/R/bin/exec/R+0x4f5512)
    #1 0x7f64ba1c208c in tbb::internal::task_stream<3>::initialize(unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/task_stream.h:94:28
    #2 0x7f64ba1c208c in tbb::internal::arena::arena(tbb::internal::market&, unsigned int, unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/arena.cpp:225:30

SUMMARY: AddressSanitizer: 3096 byte(s) leaked in 3 allocation(s).

=================================================================
==20473==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 3096 byte(s) in 3 object(s) allocated from:
    #0 0x4f5512 in operator new[](unsigned long) (/usr/local/lib/R/bin/exec/R+0x4f5512)
    #1 0x7f6a6d11b08c in tbb::internal::task_stream<3>::initialize(unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/task_stream.h:94:28
    #2 0x7f6a6d11b08c in tbb::internal::arena::arena(tbb::internal::market&, unsigned int, unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/arena.cpp:225:30

SUMMARY: AddressSanitizer: 3096 byte(s) leaked in 3 allocation(s).
clang++ -stdlib=libc++ -fsanitize=address,undefined -fno-sanitize=float-divide-by-zero -fno-omit-frame-pointer -fsanitize-address-use-after-scope -std=gnu++11 -shared -L/usr/local/lib/R/lib -L/usr/local/lib -o dodgr.so heaps/bheap.o heaps/fheap.o heaps/heap23.o heaps/radi
xheap.o heaps/triheap_ext.o heaps/triheap.o dgraph.o pathfinders.o dodgr-to-sf.o flows.o fund-cycles.o graph-contract.o graph.o graph-sample.o RcppExports.o run_sp.o sc-as-network.o sf-as-network.o turn_penalty.o -L/usr/local/lib/R/lib -lRlapack -L/usr/local/lib/R/lib -lR
blas -L/usr/lib/gcc/x86_64-linux-gnu/8 -L/usr/lib/x86_64-linux-gnu -L/lib/x86_64-linux-gnu -lgfortran -lm -lquadmath -L/usr/local/lib/R/lib -lR
installing to /tmp/Rtmpq0tZXb/Rinst4fa8322299e2/00LOCK-dodgr/00new/dodgr/libs
** R
** data
*** moving datasets to lazyload DB
** inst
** byte-compile and prepare package for lazy loading

=================================================================
==20494==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 3096 byte(s) in 3 object(s) allocated from:
    #0 0x4f5512 in operator new[](unsigned long) (/usr/local/lib/R/bin/exec/R+0x4f5512)
    #1 0x7f6a0d64808c in tbb::internal::task_stream<3>::initialize(unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/task_stream.h:94:28
    #2 0x7f6a0d64808c in tbb::internal::arena::arena(tbb::internal::market&, unsigned int, unsigned int) /tmp/Rtmpor7hGe/R.INSTALL4b68676ede42/RcppParallel/src/tbb/build/lib_release/../../src/tbb/arena.cpp:225:30

SUMMARY: AddressSanitizer: 3096 byte(s) leaked in 3 allocation(s).

So that's at least got the offending code lines in there. It would be useful for me - and presumably others - to address this, because being unable to use UBSAN checks via rocker is rather inconvenient. 😏 Should be pretty easy to reproduce by running RD CMD build on any mock package with an RcppParallel dep.

[kevinushey]

Pull requests are welcomed :-) If you're feeling brave enough to try updating us to the latest TBB release + porting all the associated patches we require to build successfully on the CRAN build machines...