Memory leak in xlsxio_read.c

[jorbakk]

I spent some time tracking down a memory leak in xlsxio_read.c but unfortunately can't provide a patch, yet. There seems to be no easy and non-intrusive fix for it and I wanted to ask for some input on how to proceed best.

It can be reproduced with a short test program that can be run with the output file example.xlsx created by examples/example_xlsxio_write.c. The test program, valgrind output, a backtrace, and details about the setup are provided below at the end of the post.

The backtrace when opening a file in the zip archive shows that unzOpenCurrentFile() is called in a recursive call of expat_process_zip_file(), first with "[Content_Types].xml", then with "xl/workbook.xml". This leads to the following call pattern when opening and closing files in the zip, which is illustrated with some printf's:

unzLocateFile(0x4d7c700, [Content_Types].xml)
unzOpenCurrentFile(0x4d7c700)
unzLocateFile(0x4d7c700, xl/workbook.xml)
unzOpenCurrentFile(0x4d7c700)
unzCloseCurrentFile(0x4d7c700)
unzLocateFile(0x4d7c700, xl/_rels/workbook.xml.rels)
unzOpenCurrentFile(0x4d7c700)
unzCloseCurrentFile(0x4d7c700)
unzCloseCurrentFile(0x4d7c700)
unzLocateFile(0x4d7c700, xl/sharedStrings.xml)
unzOpenCurrentFile(0x4d7c700)
unzCloseCurrentFile(0x4d7c700)
unzLocateFile(0x4d7c700, xl/worksheets/sheet1.xml)
unzOpenCurrentFile(0x4d7c700)
unzCloseCurrentFile(0x4d7c700)

Test program:

int main() {
	xlsxioreader reader = xlsxioread_open("example.xlsx");
	xlsxioreadersheet sheet = xlsxioread_sheet_open(reader, "MySheet", XLSXIOREAD_SKIP_EMPTY_ROWS);
	xlsxioread_sheet_close(sheet);
	xlsxioread_close(reader);
}

Valgrind output:

==78526== Memcheck, a memory error detector
==78526== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==78526== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==78526== Command: build/example
==78526==
==78526==
==78526== HEAP SUMMARY:
==78526==     in use at exit: 72,872 bytes in 3 blocks
==78526==   total heap usage: 16,543 allocs, 16,540 frees, 636,378 bytes allocated
==78526==
==78526== 72,872 (32,944 direct, 39,928 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==78526==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==78526==    by 0x1172E9: mz_stream_zlib_create (mz_strm_zlib.c:369)
==78526==    by 0x11453D: mz_zip_entry_open_int (mz_zip.c:1770)
==78526==    by 0x114928: mz_zip_entry_read_open (mz_zip.c:1901)
==78526==    by 0x10C966: unzOpenCurrentFile3 (mz_compat.c:879)
==78526==    by 0x10CA85: unzOpenCurrentFile (mz_compat.c:910)
==78526==    by 0x484F8D7: XML_Char_openzip (xlsxio_read.c:74)
==78526==    by 0x484F986: expat_process_zip_file (xlsxio_read.c:170)
==78526==    by 0x4850C0D: iterate_files_by_contenttype (xlsxio_read.c:743)
==78526==    by 0x485253C: xlsxioread_process (xlsxio_read.c:1317)
==78526==    by 0x4852CBC: xlsxioread_sheet_open (xlsxio_read.c:1465)
==78526==    by 0x10AB29: read_test_xlsx (example.c:91)
==78526==
==78526== LEAK SUMMARY:
==78526==    definitely lost: 32,944 bytes in 1 blocks
==78526==    indirectly lost: 39,928 bytes in 2 blocks
==78526==      possibly lost: 0 bytes in 0 blocks
==78526==    still reachable: 0 bytes in 0 blocks
==78526==         suppressed: 0 bytes in 0 blocks
==78526==
==78526== For lists of detected and suppressed errors, rerun with: -s
==78526== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Backtrace:

Breakpoint 1, 0x0000555555584563 in unzOpenCurrentFile ()
(gdb) bt
#0  0x0000555555584563 in unzOpenCurrentFile ()
#1  0x000055555555bec5 in XML_Char_openzip (archive=0x555555699760,
    filename=0x5555556ada66 "xl/workbook.xml", flags=0)
    at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:76
#2  0x000055555555bf74 in expat_process_zip_file (zip=0x555555699760,
    filename=0x5555556ada66 "xl/workbook.xml",
    start_handler=0x55555555d457 <main_sheet_get_relid_expat_callback_element_start>,
    end_handler=0x0, data_handler=0x0, callbackdata=0x7fffffffdd30, xmlparser=0x7fffffffdd30)
    at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:172
#3  0x000055555555d771 in main_sheet_get_sheetfile_callback (zip=0x555555699760,
    filename=0x5555556ada66 "xl/workbook.xml",
    contenttype=0x5555556ada76 "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml", callbackdata=0x7fffffffdd30) at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:874
#4  0x000055555555cf46 in iterate_files_by_contenttype_expat_callback_element_start (
    callbackdata=0x7fffffffdca0, name=0x5555556ada5c "Override", atts=0x555555699ee0)
    at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:685
#5  0x000055555556f509 in doContent ()
#6  0x0000555555570c3e in contentProcessor ()
#7  0x00005555555678b0 in XML_ParseBuffer ()
#8  0x000055555555c028 in expat_process_zip_file (zip=0x555555699760,
    filename=0x55555566124c "[Content_Types].xml",
    start_handler=0x55555555ce58 <iterate_files_by_contenttype_expat_callback_element_start>,
    end_handler=0x0, data_handler=0x0, callbackdata=0x7fffffffdca0, xmlparser=0x0)
    at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:188
#9  0x000055555555d21d in iterate_files_by_contenttype (zip=0x555555699760,
    contenttype=0x5555556610b0 "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml", filecallbackfn=0x55555555d70e <main_sheet_get_sheetfile_callback>,
    filecallbackdata=0x7fffffffdd30, xmlparser=0x0) at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:746
#10 0x000055555555eb27 in xlsxioread_process (handle=0x5555556962a0,
    sheetname=0x555555661015 "MySheet", flags=129, cell_callback=0x0, row_callback=0x0,
    callbackdata=0x555555699a10) at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:1320
#11 0x000055555555f2ca in xlsxioread_sheet_open (handle=0x5555556962a0,
    sheetname=0x555555661015 "MySheet", flags=1) at /home/jorg/cc/xlsxio/lib/xlsxio_read.c:1469
#12 0x000055555555bdea in read_test_xlsx () at memleak_test.c:91
#13 0x000055555555be1f in main () at memleak_test.c:110

My setup is:

xlsxio: current head (8d7007c82bd1a0b36bec54d9fbbf5cf3eb9210fb)
minizip_ng: 3.0.7
zlib: 1.2.13
expat: 2.5.0