RandyGaul / cute_headers

Collection of cross-platform one-file C/C++ libraries with no dependencies, primarily used for games
4.25k stars 264 forks source link

cute_tiled animations not freed #271

Open aganm opened 2 years ago

aganm commented 2 years ago

I found a memory leak happening here

https://github.com/RandyGaul/cute_headers/blob/3510be718b7657d41d05b6aa33b577e89ed13b30/cute_tiled.h#L2147

I think the respective free calls are missing

RandyGaul commented 2 years ago

Thanks for catching this!

RandyGaul commented 2 years ago

My apologies for not being super on-top of this. It's a little time consuming for me to make my own test tiled files, as I no longer use Tiled for my own game. Though, I like Tiled and want to continue supporting cute_tiled :)

Just need a JSON file to repro the bug with, and I'll have it fixed shortly thereafter!

aganm commented 2 years ago

When using an embedded tileset there's no leak, only when I use an external tileset does valgrind give me a leak here:

Direct leak of 768 byte(s) in 3 object(s) allocated from:
    #0 0x7f90985047ef in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f9097d19791 in cute_tiled_read_animation_frames_internal ../../lib/cute_headers/cute_tiled.h:2147

The way I'm loading/destroying my files is as such:

// Load
cute_tiled_map_t *map = cute_tiled_load_map_from_file("units32.json", NULL);
cute_tiled_tileset_t *tileset = map->tilesets;

cute_tiled_tileset_t *external = NULL;
if (tileset && tileset->source.ptr) { // if it has a source it means it's external
     external = cute_tiled_load_external_tileset(tileset->source.ptr, NULL);
}

// Destroy
if (external) {
    cute_tiled_free_external_tileset(external);
}
cute_tiled_free_map(map);

Here are all the files needed to reproduce: leak.zip

If I run the run.sh script on linux, this is the full output:

==16341==ERROR: LeakSanitizer: detected memory leaks 

Direct leak of 768 byte(s) in 3 object(s) allocated from:
    #0 0x7f26aad037ef in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145 
    #1 0x559b65d72e85 in cute_tiled_read_animation_frames_internal (/home/aganm/Projects/test/a.out+0xde85)
    #2 0x559b65d73a4f in cute_tiled_read_tile_descriptor (/home/aganm/Projects/test/a.out+0xea4f)
    #3 0x559b65d74e47 in cute_tiled_tileset (/home/aganm/Projects/test/a.out+0xfe47)
    #4 0x559b65d77c77 in cute_tiled_load_external_tileset_from_memory (/home/aganm/Projects/test/a.out+0x12c77)
    #5 0x559b65d77bda in cute_tiled_load_external_tileset (/home/aganm/Projects/test/a.out+0x12bda)
    #6 0x559b65d77eb3 in main (/home/aganm/Projects/test/a.out+0x12eb3)
    #7 0x7f26aaab6d09 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: 768 byte(s) leaked in 3 allocation(s).
RobLoach commented 11 months ago

I don't believe this is an issue. The frames are read to the tile descriptor's animation...

cute_tiled_read_animation_frames(m, &tile_descriptor->animation, &tile_descriptor->frame_count);

In cute_tiled_free_map_internal(), the desc->animation is freed...

if (desc->animation) CUTE_TILED_FREE(desc->animation, mem_ctx);