[API Proposal] Making the RenderTargetBitmap inherit IDisposable

[lindexi]

The RenderTargetBitmap will cature the _wicSource handler which will release slowly by GC. WPF will throw COM Exception when create RenderTargetBitmap too fast.

Making the RenderTargetBitmap inherit IDisposable. Developers can better control the release of memory.

API Proposal

1. Making the RenderTargetBitmap inherit IDisposable

public class RenderTargetBitmap : IDisposable

2. Add the AsDisposable method

public class RenderTargetBitmap
{
    public IDisposable AsDisposable(){}
}

Add the AsDisposable method can make the Analyzer happy. The Analyzer will not keep alerting that the IDisposable object may be a potential memory leak.

See https://github.com/dotnet/wpf/issues/3067#issuecomment-1065849036

[lindexi]

Or we should add the IDisposable to all BitmapSource ?

[amaitland]

In the context of CefSharp every time the WPF control is resized a new WritableBitmap is created and the old one discarded, a means to explicitly release the memory would be most welcomed. Semi frequently I get reports of OutOfMemoryException when attempting to create a WriteableBitmap.

[miloush]

Funnily I was looking into this couple of days back. BitmapSource itself has unmanaged references so it would make sense to make it IDisposable, and RTB and WB can chime in and dispose their extra references.

It needs to be said that introducing IDisposable is a breaking change, because IDisposable means "you should call dispose when you are done" and the existing code does not do that, with the worry being that unmanaged resources will leak. That is not exactly our case here, because the unmanaged resources already exist and are being taken care of during finalization. However, as @lindexi pointed out, analyzers do not know that. Another breaking change is that any classes that own BitmapSources as fields or properties should also implement IDisposable according to the guidance.

So we need to make a decision whether we are OK introducing such breaking change, or whether we want to work around that. Personally at the moment I would not be opposed too much to a breaking change here. For workarounds, I do not like the AsDisposable idea very much, because it adds extra layer of complexity and introduces a new pattern. We could have a Dispose or Close or similarly named method without actually implementing IDisposable.

There is also a 3rd option, which is to figure out why RTB and WB are running in these issues in the first place, for example, are they not adding enough GC pressure?

Finally, while this change looks easy in principle, there is couple of things that need to be figured out. Suddenly you can have disposed yet alive classes around and that has consequences. All the classes would need to be sprinkled with checks and ObjectDisposedExceptions. What happens to the cached bitmaps? What happens to bitmaps that are currently rendered on screen when they get disposed? Do we trigger content change on disposal? Do we need to worry about frozen clones? etc.

[JensNordenbro]

As an api consumer I see little problems in adding using in new places or even disable analyser warnings IF need be. As long as the changes arrive in a major version like net8!

[lindexi]

As an api consumer I see little problems in adding using in new places or even disable analyser warnings IF need be.

I think so too