s4hts / HTStream

A high throughput sequence read toolset using a streaming approach facilitated by Linux pipes
https://s4hts.github.io/HTStream/
Apache License 2.0
49 stars 9 forks source link

Issue in Common: Memory Leak in `check_open_r` (not closing file) #258

Open dstreett opened 6 months ago

dstreett commented 6 months ago

Command to Verify

valgrind --leak-check=full \
         --show-leak-kinds=all \
         --track-origins=yes \
         --verbose \
         ./hts_SuperDeduper/hts_SuperDeduper -T ../regression/hts_SuperDeduper.tab6

Output from Valgrind

==44234== HEAP SUMMARY:
==44234==     in use at exit: 472 bytes in 1 blocks
==44234==   total heap usage: 1,976 allocs, 1,975 frees, 291,733 bytes allocated
==44234== 
==44234== Searching for pointers to 1 not-freed blocks
==44234== Checked 190,608 bytes
==44234== 
==44234== 472 bytes in 1 blocks are still reachable in loss record 1 of 1
==44234==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==44234==    by 0x4BC164D: __fopen_internal (iofopen.c:65)
==44234==    by 0x4BC164D: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==44234==    by 0x1ED746: check_open_r(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /home/david/Projects/HTStream/build/hts_SuperDeduper/hts_SuperDeduper)
==44234==    by 0x1CC935: MainTemplate<SuperDeduperCounters, SuperDeduper>::main_func(int, char**) (in /home/david/Projects/HTStream/build/hts_SuperDeduper/hts_SuperDeduper)
==44234==    by 0x1C0D7C: main (in /home/david/Projects/HTStream/build/hts_SuperDeduper/hts_SuperDeduper)
==44234== 
==44234== LEAK SUMMARY:
==44234==    definitely lost: 0 bytes in 0 blocks
==44234==    indirectly lost: 0 bytes in 0 blocks
==44234==      possibly lost: 0 bytes in 0 blocks
==44234==    still reachable: 472 bytes in 1 blocks
==44234==         suppressed: 0 bytes in 0 blocks
==44234== 
==44234== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

We need to ensure we don't leave the file pointer open.

joe-angell commented 6 months ago

david any progress with this issue? we were thinking of creating a new release soon and it might be nice to have this fix in.

dstreett commented 6 months ago

I have an idea - I just need a bit of time. I am hoping to have a PR this weekend!

joe-angell commented 1 week ago

@dstreett any updates on this issue, they want to make a new release with the UMI stuff in it.