jlaurens / synctex

Synchronization for TeX
MIT License
57 stars 19 forks source link

Memory leak when file cannot be parsed #48

Closed stloeffler closed 1 year ago

stloeffler commented 1 year ago

in synctex_scanner_new_with_output_file, a scanner gets created (synctex_parser.c#L6055), but when synctex_reader_init_with_output_file fails (synctex_parser.c#L6060, e.g. because the file cannot be opened), the scanner does not get deallocated again.

Note that simply adding a synctex_scanner_free(scanner); line to synctex_scanner_new_with_output_file does not work out of the box as synctex_scanner_free dereferences scanner->reader (via SYNCTEX_FILE, synctex_parser.c#L6072-L6075), which is set to nullptr if synctex_reader_init_with_output_file fails (synctex_parser.c#L6060). One possible workaround I found is to simply remove the offending lines in synctex_scanner_free (synctex_parser.c#L6072-L6075) as the reader->file is closed (in a safer way, i.e. by first checking reader is valid) in synctex_reader_free (which is called later from synctex_scanner_free) anyway. I don't know if this affects other code, though, which might depend on closing the file early.

stloeffler commented 1 year ago

I just found a second problem in the same test case (i.e., synctex_scanner_new_with_output_file gets called with a file that cannot be read). In that case, scanner->reader is first allocated in synctex_scanner_new (synctex_parser.c#L6011) and later set to the output of synctex_reader_init_with_output_file (synctex_parser.c#L6060). Normally, synctex_reader_init_with_output_file should return scanner->reader, so the assignment is effectively a no-op. In case of a read error, however, synctex_reader_init_with_output_file returns NULL, so scanner->reader is overwritten without freeing the memory. A workaround seems to be to remove the assignment in synctex_parser.c#L6060 altogether as scanner->reader is passed as argument to synctex_reader_init_with_output_file anyway.