libjxl / libjxl

JPEG XL image format reference implementation
BSD 3-Clause "New" or "Revised" License
2.61k stars 251 forks source link

Memory leak with djxl #2509

Closed Traneptora closed 1 year ago

Traneptora commented 1 year ago

Describe the bug Valgrind reports 167 MB as "possibly lost" when decoding with libjxl. It appears to be system-dependent: I can reproduce it on my desktop but not my laptop, with the same build number.

To Reproduce

valgrind --leak-check=full djxl ./newton-cradle.jxl --disable_output

Expected behavior All Heap Blocks Are Free - No Leaks Are Possible

Environment

Additional context Log file: https://buzo.us/n.log

Traneptora commented 1 year ago

Sample: newton-cradle.jxl: https://buzo.us/3.jxl

jdpatdiscord commented 1 year ago

Unable to reproduce as of c8a4a7a, same as yours. Fedora 38 w. 6 threads, 5900X. With or without --leak-check=full is the same. Outputting to a file, no leaks, Only reading with --disable-output, no leaks. Also GCC 13.1.1.

kkourin commented 1 year ago

Based on the code and the attached log, this leak looks like it occurs when profiling is enabled (e.g. with -DJPEGXL_ENABLE_PROFILER=true). Maybe this variable set to true in the CMake cache on your laptop but not your desktop. On my machine, I tested that the leak only occurs with the profiler enabled.

From what I can tell, the memory is allocated and leaked here. https://github.com/libjxl/libjxl/blob/main/lib/jxl/base/profiler.cc#L493

And I think this comment suggests it is intentionally leaked. https://github.com/libjxl/libjxl/blob/main/lib/jxl/base/profiler.h#L107

mo271 commented 1 year ago

Probably best to remove the profiler, I don't think it is used much these days

mo271 commented 1 year ago

see #2511