[rmodels] `LoadModel()` leaks memory for .glb files with vertex colors

[TatuLaras]

Issue description

I was noticing some memory leaks that happen while loading / unloading models repeatedly.
I decided to investigate this issue and made a minimal reproduction of this bug (seen at the bottom of this issue). Increasing the iteration count in that code example proportionally leads to alarger maximum resident set. I compiled raylib with symbols and ran that example through asan, according to which the leak is from an allocation in line 5737 in rmodels.c:

if (attribute->component_type == cgltf_component_type_r_8u)
{
    // Init raylib mesh color to copy glTF attribute data
=>  model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char)); // !! OFFENDING LINE !!

    // Load 4 components of unsigned char data type into mesh.colors
    LOAD_ATTRIBUTE(attribute, 4, unsigned char, model.meshes[meshIndex].colors)
}
else if (attribute->component_type == cgltf_component_type_r_16u)

The memory leak seems to happen in relation to vertex color data.
Another fact that speaks to this is that the bug is not present when loading a .glb of a plain blender default cube, but memory starts being leaked as soon as it's exported with some vertex colors applied from the vertex paint mode.

mesh.colors is unloaded in UnloadMesh, commenting out that line leads to even more memory being leaked.
I tried to search for instances where a meshes color array pointer was changed in the course of the functions called in the code example, but couldn't find any.

Environment

OS: Arch Linux
Kernel: 6.14.3-arch1-1
CPU: Intel i5-9600K (6) @ 4.700GHz
GPU: NVIDIA GeForce RTX 2070

OpenGL device information:
> Vendor:   NVIDIA Corporation
> Renderer: NVIDIA GeForce RTX 2070/PCIe/SSE2
> Version:  3.3.0 NVIDIA 570.144
> GLSL:     3.30 NVIDIA via Cg compiler

Code Example

#include "raylib.h"

int main(int argc, char **argv) {
    InitWindow(800, 450, "Test");

    Model model;

    for (int i = 0; i < 20; i++) {
        model = LoadModel("model_with_vertex_colors.glb");
        UnloadModel(model);
    }

    CloseWindow();

    return 0;
}

Full log

INFO: Initializing raylib 5.6-dev
INFO: Platform backend: DESKTOP (GLFW)
INFO: Supported raylib modules:
INFO:     > rcore:..... loaded (mandatory)
INFO:     > rlgl:...... loaded (mandatory)
INFO:     > rshapes:... loaded (optional)
INFO:     > rtextures:. loaded (optional)
INFO:     > rtext:..... loaded (optional)
INFO:     > rmodels:... loaded (optional)
INFO:     > raudio:.... loaded (optional)
INFO: DISPLAY: Device initialized successfully
INFO:     > Display size: 1920 x 1080
INFO:     > Screen size:  800 x 450
INFO:     > Render size:  800 x 450
INFO:     > Viewport offsets: 0, 0
INFO: GLAD: OpenGL extensions loaded successfully
INFO: GL: Supported extensions count: 399
INFO: GL: OpenGL device information:
INFO:     > Vendor:   NVIDIA Corporation
INFO:     > Renderer: NVIDIA GeForce RTX 2070/PCIe/SSE2
INFO:     > Version:  3.3.0 NVIDIA 570.144
INFO:     > GLSL:     3.30 NVIDIA via Cg compiler
INFO: GL: VAO extension detected, VAO functions loaded successfully
INFO: GL: NPOT textures extension detected, full NPOT textures supported
INFO: GL: DXT compressed textures supported
INFO: GL: ETC2/EAC compressed textures supported
INFO: PLATFORM: DESKTOP (GLFW - X11): Initialized successfully
INFO: TEXTURE: [ID 1] Texture loaded successfully (1x1 | R8G8B8A8 | 1 mipmaps)
INFO: TEXTURE: [ID 1] Default texture loaded successfully
INFO: SHADER: [ID 1] Vertex shader compiled successfully
INFO: SHADER: [ID 2] Fragment shader compiled successfully
INFO: SHADER: [ID 3] Program shader loaded successfully
INFO: SHADER: [ID 3] Default shader loaded successfully
INFO: RLGL: Render batch vertex buffers loaded successfully in RAM (CPU)
INFO: RLGL: Render batch vertex buffers loaded successfully in VRAM (GPU)
INFO: RLGL: Default OpenGL state initialized successfully
INFO: TEXTURE: [ID 2] Texture loaded successfully (128x128 | GRAY_ALPHA | 1 mipmaps)
INFO: FONT: Default font loaded successfully (224 glyphs)
INFO: SYSTEM: Working Directory: /home/tatu/_repos/debug-leak
INFO: FILEIO: [../ebb/assets/t.glb] File loaded successfully
INFO: MODEL: [../ebb/assets/t.glb] Model basic data (glb) loaded successfully
INFO:     > Meshes count: 1
INFO:     > Materials count: 1 (+1 default)
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: MODEL: Unloaded model (and meshes) from RAM and VRAM
INFO: FILEIO: [../ebb/assets/t.glb] File loaded successfully
INFO: MODEL: [../ebb/assets/t.glb] Model basic data (glb) loaded successfully
INFO:     > Meshes count: 1
INFO:     > Materials count: 1 (+1 default)
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: MODEL: Unloaded model (and meshes) from RAM and VRAM
INFO: FILEIO: [../ebb/assets/t.glb] File loaded successfully
INFO: MODEL: [../ebb/assets/t.glb] Model basic data (glb) loaded successfully

...

INFO:     > Meshes count: 1
INFO:     > Materials count: 1 (+1 default)
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: MODEL: Unloaded model (and meshes) from RAM and VRAM
INFO: FILEIO: [../ebb/assets/t.glb] File loaded successfully
INFO: MODEL: [../ebb/assets/t.glb] Model basic data (glb) loaded successfully
INFO:     > Meshes count: 1
INFO:     > Materials count: 1 (+1 default)
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: MODEL: Unloaded model (and meshes) from RAM and VRAM
INFO: TEXTURE: [ID 2] Unloaded texture data from VRAM (GPU)
INFO: SHADER: [ID 3] Default shader unloaded successfully
INFO: TEXTURE: [ID 1] Default texture unloaded successfully
INFO: Window closed successfully

=================================================================
==276781==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 19200 byte(s) in 200 object(s) allocated from:
    #0 0x73b33fafd721 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x63a47c3fb49b in LoadGLTF /home/tatu/_repos/raylib/src/rmodels.c:5737
    #2 0x63a47c3deba5 in LoadModel /home/tatu/_repos/raylib/src/rmodels.c:1108
    #3 0x63a47c287a74 in main (/home/tatu/_repos/debug-leak/build/asan+0x8a74) (BuildId: 04f1efc98355ad39f2ffdaff50800c0ecfebb420)
    #4 0x73b33f835487  (/usr/lib/libc.so.6+0x27487) (BuildId: d056ce83eebe65ce7e52ecfa5af5363e4863d283)
    #5 0x73b33f83554b in __libc_start_main (/usr/lib/libc.so.6+0x2754b) (BuildId: d056ce83eebe65ce7e52ecfa5af5363e4863d283)
    #6 0x63a47c2878b4 in _start (/home/tatu/_repos/debug-leak/build/asan+0x88b4) (BuildId: 04f1efc98355ad39f2ffdaff50800c0ecfebb420)

Direct leak of 19200 byte(s) in 200 object(s) allocated from:
    #0 0x73b33fafd721 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x63a47c3fb5f5 in LoadGLTF /home/tatu/_repos/raylib/src/rmodels.c:5745
    #2 0x63a47c3deba5 in LoadModel /home/tatu/_repos/raylib/src/rmodels.c:1108
    #3 0x63a47c287a74 in main (/home/tatu/_repos/debug-leak/build/asan+0x8a74) (BuildId: 04f1efc98355ad39f2ffdaff50800c0ecfebb420)
    #4 0x73b33f835487  (/usr/lib/libc.so.6+0x27487) (BuildId: d056ce83eebe65ce7e52ecfa5af5363e4863d283)
    #5 0x73b33f83554b in __libc_start_main (/usr/lib/libc.so.6+0x2754b) (BuildId: d056ce83eebe65ce7e52ecfa5af5363e4863d283)
    #6 0x63a47c2878b4 in _start (/home/tatu/_repos/debug-leak/build/asan+0x88b4) (BuildId: 04f1efc98355ad39f2ffdaff50800c0ecfebb420)

Direct leak of 184 byte(s) in 1 object(s) allocated from:
    #0 0x73b33fafd02a in calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x73b33ed48ac3  (/usr/lib/../lib/libdbus-1.so.3+0x24ac3) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #2 0x73b33ed4abbd in _dbus_message_loader_queue_messages (/usr/lib/../lib/libdbus-1.so.3+0x26bbd) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #3 0x73b33ed558ae  (/usr/lib/../lib/libdbus-1.so.3+0x318ae) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #4 0x73b33ed55aa7  (/usr/lib/../lib/libdbus-1.so.3+0x31aa7) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #5 0x73b33ed55e00  (/usr/lib/../lib/libdbus-1.so.3+0x31e00) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #6 0x73b33ed5638c  (/usr/lib/../lib/libdbus-1.so.3+0x3238c) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #7 0x73b33ed39e92  (/usr/lib/../lib/libdbus-1.so.3+0x15e92) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #8 0x73b33ed4eb9f in dbus_pending_call_block (/usr/lib/../lib/libdbus-1.so.3+0x2ab9f) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #9 0x73b33ed3bd46 in dbus_connection_send_with_reply_and_block (/usr/lib/../lib/libdbus-1.so.3+0x17d46) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #10 0x73b33ed3bfed in dbus_bus_register (/usr/lib/../lib/libdbus-1.so.3+0x17fed) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #11 0x73b33ed3c2da  (/usr/lib/../lib/libdbus-1.so.3+0x182da) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #12 0x73b33a430326  (/usr/lib/libstdc++.so.6.0.33+0xc30326)

Indirect leak of 834 byte(s) in 1 object(s) allocated from:
    #0 0x73b33fafc3c2 in realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85
    #1 0x73b33ed57566  (/usr/lib/../lib/libdbus-1.so.3+0x33566) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #2 0x73b33ed575f4  (/usr/lib/../lib/libdbus-1.so.3+0x335f4) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #3 0x73b33ed591db in _dbus_string_copy_len (/usr/lib/../lib/libdbus-1.so.3+0x351db) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #4 0x73b33ed4b3ff in _dbus_message_loader_queue_messages (/usr/lib/../lib/libdbus-1.so.3+0x273ff) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #5 0x73b33ed558ae  (/usr/lib/../lib/libdbus-1.so.3+0x318ae) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #6 0x73b33ed55aa7  (/usr/lib/../lib/libdbus-1.so.3+0x31aa7) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #7 0x73b33ed55e00  (/usr/lib/../lib/libdbus-1.so.3+0x31e00) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #8 0x73b33ed5638c  (/usr/lib/../lib/libdbus-1.so.3+0x3238c) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #9 0x73b33ed39e92  (/usr/lib/../lib/libdbus-1.so.3+0x15e92) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #10 0x73b33ed4eb9f in dbus_pending_call_block (/usr/lib/../lib/libdbus-1.so.3+0x2ab9f) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #11 0x73b33ed3bd46 in dbus_connection_send_with_reply_and_block (/usr/lib/../lib/libdbus-1.so.3+0x17d46) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #12 0x73b33a4929b3  (/usr/lib/libstdc++.so.6.0.33+0xc929b3)

Indirect leak of 88 byte(s) in 1 object(s) allocated from:
    #0 0x73b33fafc3c2 in realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85
    #1 0x73b33ed57566  (/usr/lib/../lib/libdbus-1.so.3+0x33566) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #2 0x73b33ed575f4  (/usr/lib/../lib/libdbus-1.so.3+0x335f4) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #3 0x73b33ed591db in _dbus_string_copy_len (/usr/lib/../lib/libdbus-1.so.3+0x351db) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #4 0x73b33ed4abfd in _dbus_message_loader_queue_messages (/usr/lib/../lib/libdbus-1.so.3+0x26bfd) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #5 0x73b33ed558ae  (/usr/lib/../lib/libdbus-1.so.3+0x318ae) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #6 0x73b33ed55aa7  (/usr/lib/../lib/libdbus-1.so.3+0x31aa7) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #7 0x73b33ed55e00  (/usr/lib/../lib/libdbus-1.so.3+0x31e00) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #8 0x73b33ed5638c  (/usr/lib/../lib/libdbus-1.so.3+0x3238c) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #9 0x73b33ed39e92  (/usr/lib/../lib/libdbus-1.so.3+0x15e92) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #10 0x73b33ed4eb9f in dbus_pending_call_block (/usr/lib/../lib/libdbus-1.so.3+0x2ab9f) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #11 0x73b33ed3bd46 in dbus_connection_send_with_reply_and_block (/usr/lib/../lib/libdbus-1.so.3+0x17d46) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #12 0x73b33ed3bfed in dbus_bus_register (/usr/lib/../lib/libdbus-1.so.3+0x17fed) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #13 0x73b33ed3c2da  (/usr/lib/../lib/libdbus-1.so.3+0x182da) (BuildId: 65f44f39f68b8f63de82c45fc6108bfc66c0247a)
    #14 0x73b33a430326  (/usr/lib/libstdc++.so.6.0.33+0xc30326)

SUMMARY: AddressSanitizer: 39506 byte(s) leaked in 403 allocation(s).

[TatuLaras]

Narrowed it down a bit more. Changing this:

// rmodels:5734
if (attribute->component_type == cgltf_component_type_r_8u)
{
    // Init raylib mesh color to copy glTF attribute data
    model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char));

    // Load 4 components of unsigned char data type into mesh.colors
    LOAD_ATTRIBUTE(attribute, 4, unsigned char, model.meshes[meshIndex].colors)
}
else if (attribute->component_type == cgltf_component_type_r_16u)
{
    // Init raylib mesh color to copy glTF attribute data
    model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char));

    // Load data into a temp buffer to be converted to raylib data type
    unsigned short *temp = (unsigned short *)RL_MALLOC(attribute->count*4*sizeof(unsigned short));
    LOAD_ATTRIBUTE(attribute, 4, unsigned short, temp);

    // Convert data to raylib color data type (4 bytes)
    for (unsigned int c = 0; c < attribute->count*4; c++) model.meshes[meshIndex].colors[c] = (unsigned char)(((float)temp[c]/65535.0f)*255.0f);

    RL_FREE(temp);
}
else if (attribute->component_type == cgltf_component_type_r_32f)
{
    // Init raylib mesh color to copy glTF attribute data
    model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char));

    // Load data into a temp buffer to be converted to raylib data type
    float *temp = (float *)RL_MALLOC(attribute->count*4*sizeof(float));
    LOAD_ATTRIBUTE(attribute, 4, float, temp);

    // Convert data to raylib color data type (4 bytes), we expect the color data normalized
    for (unsigned int c = 0; c < attribute->count*4; c++) model.meshes[meshIndex].colors[c] = (unsigned char)(temp[c]*255.0f);

    RL_FREE(temp);
}

Into this:

if (attribute->component_type == cgltf_component_type_r_16u)
{
    // Init raylib mesh color to copy glTF attribute data
    model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char));

    // Load data into a temp buffer to be converted to raylib data type
    unsigned short *temp = (unsigned short *)RL_MALLOC(attribute->count*4*sizeof(unsigned short));
    LOAD_ATTRIBUTE(attribute, 4, unsigned short, temp);

    // Convert data to raylib color data type (4 bytes)
    for (unsigned int c = 0; c < attribute->count*4; c++) model.meshes[meshIndex].colors[c] = (unsigned char)(((float)temp[c]/65535.0f)*255.0f);

    RL_FREE(temp);
}
else if (attribute->component_type == cgltf_component_type_r_32f)
{
    // Init raylib mesh color to copy glTF attribute data
    model.meshes[meshIndex].colors = (unsigned char *)RL_MALLOC(attribute->count*4*sizeof(unsigned char));

    // Load data into a temp buffer to be converted to raylib data type
    float *temp = (float *)RL_MALLOC(attribute->count*4*sizeof(float));
    LOAD_ATTRIBUTE(attribute, 4, float, temp);

    // Convert data to raylib color data type (4 bytes), we expect the color data normalized
    for (unsigned int c = 0; c < attribute->count*4; c++) model.meshes[meshIndex].colors[c] = (unsigned char)(temp[c]*255.0f);

    RL_FREE(temp);
}

(Aka. removing the first if-block) makes it so that vertex colors still work but no memory is being leaked.
The memory is leaked when the allocation in one of the other two if-blocks overwrites the .colors buffer pointer allocated in the first one.

[JeffM2501]

If you remove that first block, how do the colors get copied from the u8 buffer into the raylib managed memory? This seems bad. We can't just use the attribute buffer in the mesh structure, the gltf library will deallocate it when it is unloaded.

If there is a leak when there is an overwrite, the correct fix is to check the pointer fist and deallocate it before reallocating it, not just skipping the one case that happens to cause the double allocation.

[JeffM2501]

Please try PR #4998

[JeffM2501]

I have inspected the file that causes this issue.

The GLTF file has two sets of color attributes for the same mesh node.
One at 8 bits per pixel and one at 16 bits per pixel.
The current code assumes that there will only ever be one attribute for each attribute type, so it does not check if the buffer exists before allocating it.
This is not the case in this file.
PR #4998 forces the loader to check if the buffer exists, and deallocate it before allocating a new one. This will prevent the leak in this case with files like this.

The suggested code above that skips the 8 bit color read is incorrect, and will cause colors to not be loaded on files that only have an 8 bit color attribute.

My guess is that some tools that support vertex colors that have more than 8 bits per pixel also output an 8 bit version for compatibility with applications that can only read 8 bit colors, and this is a situation that the current reader can not support properly.

[raysan5]

@JeffM2501 I would just load the first set found and skip subsequend sets if present, maybe with a WARNING log.

It will avoid freeing and re-loading memory for all multiple attributes found. It's up to the user to make sure the provided model is valid enough; maybe the user doesn't knows the buffer wanted, it could be just the first or the second of the third.