Handling unmanaged resources with Dispose & Finalize

C#, Performance, Unmanaged, GC, SharpDX, Architectural pattern,

Hi Everyone.
We all work in a managed world we do not control objects' lifetime do not worry about memory allocation (in most cases). We have GC and it does its work excellent. But the unmanaged word still exists and often we forced to interact with it. Let's talk a bit about unmanaged resources and managed approaches to acting with it.

As we all know implementation IDisposable does not guarantee cleaning our object automatically. It is just a contract that shows to the client that an object must be cleaned manually. I mean using statement to ensures the correct use of IDisposable or try/finally

The client is able to forget to invoke Dispose() and we need to guarantee cleaning we should implement the finalization. There is the best approach to do that MSDN.

I don't want to talk about an easy case like this:

using(var stream = File.Open()){ ... }

I want to make out more complex cases when you need to keep many objects for a while and handle recreating and not lose control of resources.

Precondition

As an instance, I take DirectX render component class.

class RenderComponent : IDisposable{
    public Direct3D11.Buffer VertexBuffer { get; set; }
    public Direct3D11.Buffer IndexBuffer { get; set; }
    
    bool disposed;
    ~RenderComponent(){
        Dispose();
    }
    public void Dispose() {
      if (disposed) {
        return;
      }
      VertexBuffer?.Dispose();
      IndexBuffer?.Dispose();
      
      disposed = true;
    }    
}

It looks kind of OK but ... What does happen if the client uses set for a Buffer?

renderCom.VertexBuffer = new Direct3D11.Buffer();

This is a problem client does not expect some extra action with a simple setting value. The client should be aware that just a set value to the property is not correct and the correct setting must be as following.

renderCom.VertexBuffer.Dispose();
renderCom.VertexBuffer = new Direct3D11.Buffer();

We have several ways to solve this problem.

try to transfer knowledge to the team
look after all changes from code review

All of these every one of us does and still, there is no guarantee because everybody makes mistakes. Furthermore, no one likes to spend time on boring tasks such as reviewing code is littered with typos. People make more mistakes working with boring tasks.

Architecture should not allow doing something wrong it must exclude the chance to shoot yourself in the foot. The solution above is too fragile to use it in the team.

Protect from losing Dispose() while setting

class RenderComponent : IDisposable{
    Direct3D11.Buffer vertexBuffer;
    public Direct3D11.Buffer VertexBuffer {
     get => vertexBuffer;
     set {
         vertexBuffer?.Dispose();
         vertexBuffer = value;
     }
    }
    public Direct3D11.Buffer IndexBuffer { ... }
}

It works, it simple and fast to implement. There is only one problem - will be a lot of duplicate code for the next example.

class RenderComponent : IDisposable{
    Direct3D11.Buffer vertexBuffer;
    Direct3D11.Buffer indexBuffer;
    public Direct3D11.Buffer VertexBuffer {
     get => vertexBuffer;
     set {
         vertexBuffer?.Dispose();
         vertexBuffer = value;
     }
    }
    public Direct3D11.Buffer IndexBuffer {
     get => indexBuffer;
     set {
         indexBuffer?.Dispose();
         indexBuffer = value;
     }
    }
    public Direct3D11.VertexShader VertexShader { ... }
    public Direct3D11.PixelShader PixelShader { .. }
    //etc...
}

As you can see for each new property, we should write the same block code, not good! Let's reduce duplication and create Decorator class for that.

Disposable setter

public class DisposableSetter<T> : IDisposable where T : IDisposable {
  T disposable;
  bool disposed;
  
  ~DisposableSetter() {
    Dispose();
  }
  
  public bool HasValue => disposable != null;
  
  public void Dispose() {
    if (disposed) {
      return;
    }
    disposable?.Dispose();
    disposable = null;
    disposed = true;
  }
  
  public T Get() => disposable;
  public void Set(T b) {
    disposable?.Dispose();
    disposable = b;
  }

class RenderComponent {
  public DisposableSetter<Direct3D11.Buffe> VertexBuffer { get; }
  public DisposableSetter<Direct3D11.Buffer> IndexBuffer { get; }
  public DisposableSetter<Direct3D11.VertexShader> VertexShader { get; }
  public DisposableSetter<Direct3D11.PixelShader> PixelShader { get; }
  //...
  public RenderComponent() {
    VertexBuffer = new DisposableSetter<Direct3D11.Buffer>();
    IndexBuffer = new DisposableSetter<Direct3D11.Buffer>();
    //...
  }
}

Let's analyze what we got.

All properties is readonly and created by default this decreases conditions to NULL, like this
```
if(renderCom.VertexBuffer != null)
```
and compilation will invoke error in that case
```
renderCom.VertexBuffer = null;
```
Disposing will be invoked automatically in setting property by
```
renderCom.VertexBuffer.Set(new Direct3D11.Buffer());
```
client does not worry about it.

Now properties look neat and readable but still, we have Dispose that will increase when the new property will be added.

class RenderComponent : IDisposable{
  //...
    protected virtual void Dispose() {
      if (disposed) {
        return;
      }
      //we do not need to check by .? there is not possible to get NULL
      VertexBuffer.Dispose();
      IndexBuffer.Dispose();
      VertexShader.Dispose();
      PixelShader.Dispose();
      //etc.
        
      disposed = true;
    }
}

Dispose watcher

In general, it is not so big deal to add many lines to Dispose(). As some smart people say "Duplication is better than the wrong abstraction". I know for sure that duplication is able to hide code mistakes, I saw that many times. Let's have a look at this.

  VertexBuffer.Dispose();
  IndexBuffer.Dispose();
  VertexBuffer.Dispose();
  PixelShader.Dispose();
  ...

Do you see the problem? Third Dispose is copy/paste of first one this mistake is easy to skip on code review and this is very frequent mistake. It is annoying. Let's change.

Create a watcher that will know about all disposable objects and able to dispose them.

public class DisposeWatcher : IDisposable {
  readonly List<IDisposable> disposable; 
  bool disposed;
  
  ~DisposeWatcher() {
    Dispose();
  }
  public DisposeWatcher() {
    disposable = new List<IDisposable>();
  }
  public void Dispose() {
    if (disposed) { return; }
   
    foreach (var d in disposable) {
       d.Dispose();
    }
    disposable.Clear();
    
    disposed = true;  
  }
  public void Watch(IDisposable dis) {
    disposable.Add(dis);
  }
}

Also we need to change a bit setter. Add dependency to the constructor to disallow the possibility to create it without a watcher.

public class DisposableSetter<T> : IDisposable where T : IDisposable {
  public DisposableSetter(DisposeWatcher watcher) {
    watcher.Watch(this);
  }
}

There is a final result. We reduced code duplication and not so increase complexity.

class RenderComponent{
  public DisposableSetter<Direct3D11.Buffer> VertexBuffer { get; }
  public DisposableSetter<Direct3D11.Buffer> IndexBuffer { get; }
  public DisposableSetter<Direct3D11.VertexShader> VertexShader { get; }
  public DisposableSetter<Direct3D11.PixelShader> PixelShader { get; }
  //etc ...
  readonly DisposeWatcher watcher;
  bool disposed;
  
  ~RenderComponent() {
    Dispose();
  }
  
  public RenderComponent(){
    watcher = new DisposeWatcher();
    VertexBuffer = new DisposableSetter<Direct3D11.Buffer>(watcher);
    IndexBuffer = new DisposableSetter<Direct3D11.Buffer>(watcher);
    //etc ...
  }
  
  public void Dispose() {
    if (disposed) { return; }
    watcher.Dispose();      
   
    disposed = true;  
  }
  }

Disposable collection

And .. it still is not the end. Often I have collection of similar disposable objects and hadlindling looks like following;

class RenderComponent {
  public IEnumerable<ShaderResourceView> TextureResources { get; set; }
  public void Dispose(){
    foreach(var t in TextureResources){
      t.Dispose();
    }
    watcher.Dispose();
  }
}

Looking at all above approaches we can simplify collection usage by creating decorator similar to DisposableSetter.

class EnumerableDisposableSetter<T> : IDisposable where T : IEnumerable<IDisposable> {
  T disposable;
  bool disposed;
  
  ~EnumerableDisposableSetter() {
    Dispose();
  }
  
  public EnumerableDisposableSetter(DisposeWatcher watcher) {
    watcher.Watch(this);
  }
  
  public void Dispose() {
   if (disposed) {
      return;
    }
    DisposeEnumerable();    
    disposed = true;
  }
  
  public T Get() => disposable;
  public bool HasValue => disposable != null && disposable.Any();
  
  public void Set(T b) {
    DisposeEnumerable();
    disposable = b;
  }
  
  void DisposeEnumerable(){
    if(HasValue) {
      foreach (var d in disposable) {
         d.Dispose();
       }
       disposable = null;
     }
  }
}

Summarise all changes

DisposableSetter self controls clearing prev object before setting a new value .
EnumerableDisposableSetter does the same as DisposableSetter but for collection
DisposeWatcher reduces duplication and cleans up all disposable objects at a time.
Abstraction has restricted usage that does not allow clients to do something wrong
Semantic of classes/methods look clean and easy to use, I hope so :)

Sorry for many lines of codes I just wanted to demonstrate all cases that I had. Of course, it is overengineering if we have one disposable object in that case all the pros come to naught.

Thanks a lot. Bye!