double free from Meshes in Model

[furudbat]

When loading the Models with Mesh the ownership is not clear and the mash(es)(?) got double freed.

Logs

INFO: TEXTURE: [ID 2] Texture loaded successfully (128x128 | GRAY_ALPHA | 1 mipmaps)
INFO: FONT: Default font loaded successfully (224 glyphs)
INFO: FILEIO: [resources/cubicmap.png] File loaded successfully
INFO: IMAGE: Data loaded successfully (32x16 | R8G8B8 | 1 mipmaps)
INFO: TEXTURE: [ID 3] Texture loaded successfully (32x16 | R8G8B8 | 1 mipmaps)
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: FILEIO: [resources/cubicmap_atlas.png] File loaded successfully
INFO: IMAGE: Data loaded successfully (256x256 | R8G8B8A8 | 1 mipmaps)
INFO: TEXTURE: [ID 4] Texture loaded successfully (256x256 | R8G8B8A8 | 1 mipmaps)
INFO: TIMER: Target time per frame: 16.667 milliseconds
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
double free or corruption (!prev)
INFO: Window closed successfully
INFO: TEXTURE: [ID 4] Unloaded texture data from VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)

models example

    // Define the camera to look into our 3d world
    raylib::Camera camera({ 0.2f, 0.4f, 0.2f }, { 0.0f, 0.0f, 0.0f }, { 0.0f, 1.0f, 0.0f }, 45.0f);

    raylib::Texture cubicmap;
    raylib::Model model;
    raylib::Texture texture;

    // load from imMap
    {
        raylib::Image imMap;
        imMap.Load("resources/cubicmap.png");      // Load cubicmap image (RAM)
        cubicmap.Load(imMap);                    // Convert image to texture to display (VRAM)
        raylib::Mesh mesh = raylib::Mesh::Cubicmap(imMap, Vector3{1.0f, 1.0f, 1.0f});
        model.Load(mesh);

        // NOTE: By default each cube is mapped to one part of texture atlas
        texture.Load("resources/cubicmap_atlas.png");    // Load map texture
        model.materials[0].maps[MATERIAL_MAP_DIFFUSE].texture = texture;     // Set map diffuse texture
    }


// Error happen at the end of the main
    ~Model() {
        Unload(); ///< error here ::UnloadModel(*this);
    }

rmodel.c

// Unload model (meshes/materials) from memory (RAM and/or VRAM)
// NOTE: This function takes care of all model elements, for a detailed control
// over them, use UnloadMesh() and UnloadMaterial()
void UnloadModel(Model model)
{
    // Unload meshes
    for (int i = 0; i < model.meshCount; i++) UnloadMesh(model.meshes[i]); ///< here

...

// Unload mesh from memory (RAM and VRAM)
void UnloadMesh(Mesh mesh)
{
    // Unload rlgl mesh vboId data
    rlUnloadVertexArray(mesh.vaoId);

    if (mesh.vboId != NULL) for (int i = 0; i < MAX_MESH_VERTEX_BUFFERS; i++) rlUnloadVertexBuffer(mesh.vboId[i]);
    RL_FREE(mesh.vboId);

    RL_FREE(mesh.vertices); ///< error here

[furudbat]

For Context, LoadModelFromMesh from rmodels.c:

// Load model from generated mesh
// WARNING: A shallow copy of mesh is generated, passed by value,
// as long as struct contains pointers to data and some values, we get a copy
// of mesh pointing to same data as original version... be careful!
Model LoadModelFromMesh(Mesh mesh)
{
    Model model = { 0 };

    model.transform = MatrixIdentity();

    model.meshCount = 1;
    model.meshes = (Mesh *)RL_CALLOC(model.meshCount, sizeof(Mesh));
    model.meshes[0] = mesh; ///< here

Seems like the mesh ownership is shared with the model.

[RobLoach]

Good catch 👍 What would be the best way around this? Something similar to the TextureUnmanaged approach we took?

[furudbat]

I'm honestly not sure, my first though would be a Mesh and SharedMesh class, the SharedMesh can only been used by the Model. (Maybe something like a shared_ptr), but idk.
The SharedMesh has a "weak-ptr" to the Model(s) and know when the Model has been unloaded, so it didn't need to unload (itself). (but that's just to much shared memory management 💀 IMO)

Maybe a MeshUnmanaged can be fine for the first solution.

I also had look into RenderTexture and they have similar ownership problem, I changed my own code to this:

    void SetTexture(const ::Texture& newTexture) = delete;
    void SetTexture(::Texture&& newTexture) {
        texture = newTexture;
        newTexture = NullTexture;
    }

rtextures.c

// Unload render texture from GPU memory (VRAM)
void UnloadRenderTexture(RenderTexture2D target)
{
    if (target.id > 0)
    {
        if (target.texture.id > 0)
        {
            // Color texture attached to FBO is deleted
            rlUnloadTexture(target.texture.id);
        }

        // NOTE: Depth texture/renderbuffer is automatically
        // queried and deleted before deleting framebuffer
        rlUnloadFramebuffer(target.id);
    }
}

texture in RenderTexture gets unloaded.