Description

Context: https://learn.microsoft.com/en-us/archive/blogs/cbrumme/lifetime-gc-keepalive-handle-recycling
Context: unoplatform/uno#21660
Context: dotnet/java-interop#719

Background: The most important background is Chris Bruce's blog post Lifetime, GC.KeepAlive, handle recycling. Excerpts:

class C {
   IntPtr _handle;
   Static void OperateOnHandle(IntPtr h) { ... }
   void m() {
      OperateOnHandle(_handle);
      ...
   }
   ...
}
 
class Other {
   void work() {
      if (something) {
         C aC = new C();
         aC.m();
         ...  // most guess here
      } else {
         ...
      }
   }
}

It’s more interesting to worry about the earliest point that aC could be collected. …

… Actually, aC could become eligible for collection before C.m() even calls C.OperateOnHandle(). Once we’ve extracted _handle from this, there are no further uses of this object. In other words, this can be collected even while you are executing an instance method on that object.

But what if class C has a Finalize() method which closes _handle? When we call C.OperateOnHandle(), we now have a race between the application and the GC / Finalizer. Eventually, that’s a race we’re going to lose.

Rephrased:

1. .NET is a multi-threaded environment, and the GC is one of many threads that can mutate your data.
2. The .NET runtime cannot "see" into native code
3. Therefore, care must be taken to ensure that we don't allow the GC to collect instances while native code is operating on them.

dotnet/java-interop#719 was one non-Skia example of this.

unoplatform/uno#21660 is a Skia-related example of this. Paraphrasing greatly, we have:

var canvas = …
var picture = …
canvas.DrawPicture(picture);
// `picture` is not referenced after this point.

While we may wish that the canvas.DrawPicture() call will keep picture alive, it does not. Eventually you hit:

meaning once exeecution hits SkiaApi.sk_canvas_draw_picture(), nothing ensures that picture is kept alive. (Or canvas, for that matter!)

The fix: audit the code (is there a generator?) such that all reference type parameters are explicitly kept alive across P/Invoke boundaries. For example, SKCanvas.DrawPicture() should be updated to be:

partial class SKCanvas {
	public void DrawPicture (SKPicture picture, SKPaint paint = null)
	{
		if (picture == null)
			throw new ArgumentNullException (nameof (picture));
		SkiaApi.sk_canvas_draw_picture (Handle, picture.Handle, null, paint == null ? IntPtr.Zero : paint.Handle);
		GC.KeepAlive (picture);
		GC.KeepAlive (paint);
		// *arguably* GC.KeepAlive(this), but maybe just require that the caller keep it around?
	}
}

This should be done for all public methods that invoke P/Invoke methods.

Code

Expected Behavior

No response

Actual Behavior

No response

Version of SkiaSharp

3.116.0 (Current)

Last Known Good Version of SkiaSharp

Other (Please indicate in the description)

IDE / Editor

Visual Studio Code (macOS)

Platform / Operating System

Android, All

Platform / Operating System Version

No response

Devices

No response

Relevant Screenshots

No response

Relevant Log Output

Code of Conduct

• I agree to follow this project's Code of Conduct