2

Consider this class for a 3D model. I can have multiple instances of the class. I am able to add, and remove my 3D models in the scene.

class My3DModel
{
    int VertexArrayObject;
    int VertexBufferObject;
    int ShaderProgram;

    int UniformLocationColor;

    public My3DModel()
    {
        VertexArrayObject = GL.GenVertexArray();
        VertexBufferObject = GL.GenBuffer();
        ShaderProgram = BuildShaders();

        UniformLocationColor = GL.GetUniformLocation(ShaderProgram, "uniColor");
    }

    ~My3DModel()
    {
        // [1] In destructor, I'm deleting the VAO, VBO, and Shader program.
        // This is all I'm using, no textures, no extra buffer objects used.
        GL.DeleteBuffer(VertexBufferObject);
        GL.DeleteVertexArray(VertexArrayObject);
        GL.DeleteProgram(ShaderProgram);
    }

    // Render function, etc. Shaders and rendering work fine
}

How I build my shaders:

int BuildShaders()
{
        int iPgm = GL.CreateProgram();

        int VertexShader = CreateAndCompileVS();
        int FragmentShader = CreateAndCompileFS();

        GL.AttachShader(iPgm, VertexShader);
        GL.AttachShader(iPgm, FragmentShader);

        GL.LinkProgram(iPgm);

        // [2] After linking program, we detach the two shaders and delete them
        GL.DetachShader(iPgm, VertexShader);
        GL.DeleteShader(VertexShader);
        GL.DetachShader(iPgm, FragmentShader);
        GL.DeleteShader(FragmentShader);

        return iPgm;
}

To test for memory leaks, I'm using a loop where I create a new instance of the class, render it, then immediately assign it to null, however my RAM usage goes up. This happens only when doing this, which is what brought me to this conclusion. This may not be a specific case for OpenTK, but I am currently convinced that it is, since I do not get this problem with memory when I strip the class from its Buffer objects and shaders and regularly use its members.

void RenderLoop()
{
    My3DModel temp_model = new My3DModel();
    // Note that I'm not pushing any data to the vertex buffer, I'm only calling the render func for the sake of it. Despite this, RAM still goes up!
    temp_model.Render();
    temp_model = null; // Even without doing this, my RAM goes up.
}

What causes this exactly, and what am I missing in the destructor, if there is something I am missing?

I am using OpenTK 3.3.1, which is the latest supported OpenTK for WinForms using .NET Framework 4.7.2, and I do not intend to migrate my project to WPF, for the record.

IOviSpot
  • 358
  • 3
  • 19
  • 1
    Setting a variable to `null` doesn't immediately release memory. The GC scans periodically for unreferenced object instances. It will release it whenever it does, or *not at all* if there's not enough pressure. Also, C# doesn't have destructors. It uses the C++ destructor *syntax* for finalizers (the `~ClassName` member overrides a method called `Finalize`), and having a finalizer can actually make the instance stick around *longer* because the GC has to coordinate finalization (pause all threads, run finalizers, resume threads) before it can release the memory. – madreflection Jul 30 '23 at 15:09
  • Is there any way to pressure GC into taking care of said object as soon as I'm done using it in the loop, without causing problems? I'm not only using a continuous cycle (loop) for testing the memory leak, I may actually need to practice the same thing for the final product, which is beyond me. – IOviSpot Jul 30 '23 at 15:14
  • 2
    The *"removing a fly from a friend's forehead with an axe"* way to is to call `GC.Collect()`; in other words, it's the nuclear option. It comes with its own costs since *you* can't know if it's optimal to collect memory at that time. Don't do it. The Disposable pattern gives you control over when *unmanaged* resources are released. There have been immense amounts written about it, so I won't go into it. Read up. If you want tight control over memory usage, though, a managed language/runtime is not what you need. – madreflection Jul 30 '23 at 15:18
  • 2
    First, make your class disposable (i.e., implement `IDisposable`). Then `Dispose` your instances when you are finished with them (and, in your Dispose method, make sure you call `GC.SupressFinalization(this)` - you really don't want your finalizer to run if you can prevent it. Then, you say "however my RAM usage goes up." How are you measuring that. Use _Performance Monitor_ (aka _PerfMon_) with the .NET Memory Counters to do this. Finally, if you are going to have a Finalizer, be _really_ careful; you must not access any managed objects in you Finalizer code, they may no longer exist – Flydog57 Jul 30 '23 at 17:25
  • Yes! This is exactly what I've read and implemented in my code right away. I'm currently on my phone so I will update my question with the solution later. Implementing `IDisposable`, its `Dispose` method, and calling `GC.SupressFinalization(this)` prevented the memory leak. I would obviously call My3DModel.Dispose in my calling functions, when I want to actually delete them from my program. – IOviSpot Jul 30 '23 at 18:41
  • 1
    One tricky thing about the Disposable pattern is that implementing it can have a cascading effect. Anything that uses a disposable object for longer than the duration of a method itself needs to be disposable (if you're storing it in a field, for example, so it can be used by multiple methods), which then could have the same effect on anything that uses *that*. – madreflection Jul 30 '23 at 19:17
  • 1
    The other thing to remember is that the Dispose pattern uses code you write and you call. There's no _magic_ in it (where there is some magic involved in the GC and in finalizers). The `using` keyword helps, but it's even more manual than RAII in C++ (well, with `using` it's close) – Flydog57 Jul 30 '23 at 21:16

1 Answers1

1

If you have a finalizer, ~My3DModel(), it does not mean the object will be removed immediately.

Additionally, the finalization is not determined and this causes some drawbacks in terms of time to free the memory.

Please consider using IDisposable instead.

Denis Kiryanov
  • 451
  • 3
  • 9