png reader leaks fd if invalid png is read and user_error_fn is hit

[springmeyer]

This problem was exposed by the invalid png at #1782. I saw that repeated map loads with this invalid png lead to "too many files open" errors. valgrind --leak-check=full ./tests/cpp_tests/image_io_test-bin should also show the leak. jpeg/tiff reader also may suffer from the same issue.

[springmeyer]

fd leaking in b6ce472, but we are still also leaking structs:

==79064== 16,536 (1,192 direct, 15,344 indirect) bytes in 1 blocks are definitely lost in loss record 95 of 96
==79064==    at 0x5227: malloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==79064==    by 0x466EEA: png_create_struct_2 (in /opt/libpng-1.5.8/lib/libpng15.15.dylib)
==79064==    by 0x46959A: png_create_read_struct_2 (in /opt/libpng-1.5.8/lib/libpng15.15.dylib)
==79064==    by 0x46954A: png_create_read_struct (in /opt/libpng-1.5.8/lib/libpng15.15.dylib)
==79064==    by 0x8E4278: mapnik::png_reader::init() (in /usr/local/lib/libmapnik.dylib)
==79064==    by 0x8E407A: mapnik::(anonymous namespace)::create_png_reader(std::string const&) (in /usr/local/lib/libmapnik.dylib)
==79064==    by 0x1000011DF: main (in tests/cpp_tests/image_io_test-bin)

==79314== 312 bytes in 1 blocks are definitely lost in loss record 61 of 93
==79314==    at 0x5227: malloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==79314==    by 0x466EEA: png_create_struct_2 (in /opt/libpng-1.5.8/lib/libpng15.15.dylib)
==79314==    by 0x461F84: png_create_info_struct (in /opt/libpng-1.5.8/lib/libpng15.15.dylib)
==79314==    by 0x8E42A1: mapnik::png_reader::init() (in /usr/local/lib/libmapnik.dylib)
==79314==    by 0x8E406A: mapnik::(anonymous namespace)::create_png_reader(std::string const&) (in /usr/local/lib/libmapnik.dylib)
==79314==    by 0x1000011DF: main (in tests/cpp_tests/image_io_test-bin)

[springmeyer]

still have leaking in tiff and jpeg:

==1683== 1,042 bytes in 1 blocks are definitely lost in loss record 2 of 8
==1683==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1683==    by 0x7786A82: TIFFClientOpen (in /usr/lib/x86_64-linux-gnu/libtiff.so.4.3.4)
==1683==    by 0x7790F78: TIFFFdOpen (in /usr/lib/x86_64-linux-gnu/libtiff.so.4.3.4)
==1683==    by 0x7790FE7: TIFFOpen (in /usr/lib/x86_64-linux-gnu/libtiff.so.4.3.4)
==1683==    by 0x5845D6F: mapnik::tiff_reader::load_if_exists(std::string const&) (in /usr/local/lib/libmapnik.so.2.2.0)
==1683==    by 0x5846313: mapnik::tiff_reader::init() (in /usr/local/lib/libmapnik.so.2.2.0)
==1683==    by 0x5846628: mapnik::tiff_reader::tiff_reader(std::string const&) (in /usr/local/lib/libmapnik.so.2.2.0)
==1683==    by 0x5846698: mapnik::(anonymous namespace)::create_tiff_reader(std::string const&) (in /usr/local/lib/libmapnik.so.2.2.0)
==1683==    by 0x56F9923: mapnik::get_image_reader(std::string const&, std::string const&) (in /usr/local/lib/libmapnik.so.2.2.0)
==1683==    by 0x402088: main (in /home/ubuntu/mapnik/tests/cpp_tests/image_io_test-bin)

==79553== 16 bytes in 1 blocks are definitely lost in loss record 8 of 92
==79553==    at 0x58D3: calloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==79553==    by 0x1B10A62: __cxa_get_globals (in /usr/lib/libc++abi.dylib)
==79553==    by 0x1B1156E: __cxa_throw (in /usr/lib/libc++abi.dylib)
==79553==    by 0x8E2EBF: mapnik::jpeg_reader::on_error(jpeg_common_struct*) (in /usr/local/lib/libmapnik.dylib)
==79553==    by 0x4A2E54: fill_input_buffer (in /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib)
==79553==    by 0x4A8A88: read_markers (in /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib)
==79553==    by 0x4A74D0: consume_markers (in /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib)
==79553==    by 0x4A1021: jpeg_consume_input (in /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib)
==79553==    by 0x4A0F8C: jpeg_read_header (in /usr/local/Cellar/jpeg/8d/lib/libjpeg.8.dylib)
==79553==    by 0x8E29CE: mapnik::jpeg_reader::init() (in /usr/local/lib/libmapnik.dylib)
==79553==    by 0x8E27B8: mapnik::jpeg_reader::jpeg_reader(std::string const&) (in /usr/local/lib/libmapnik.dylib)
==79553==    by 0x8E2741: mapnik::(anonymous namespace)::create_jpeg_reader(std::string const&) (in /usr/local/lib/libmapnik.dylib)

[springmeyer]

jpeg leak looks spurious. replicated tiff leak on ubuntu as well. It looks like if the TIFFSetErrorHandler is set then memory is not properly deallocated in a failed load and tiff_closer when called has a null pointer. Perhaps a bug in tiff? Anyway, should be worked around now.

[springmeyer]

cannot replicate jpeg leak on ubuntu. closing.

[artemp]

@springmeyer - nm, got it:)