Resolve some concurrency issues

binding/Binding/HandleDictionary.cs

@@ -0,0 +1,215 @@
+using System;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Threading;
+
+namespace SkiaSharp
+{
+	internal static class HandleDictionary
+	{
+		private static readonly Type IntPtrType = typeof (IntPtr);
+		private static readonly Type BoolType = typeof (bool);
+
+#if THROW_OBJECT_EXCEPTIONS
+		internal static readonly ConcurrentBag<Exception> exceptions = new ConcurrentBag<Exception> ();
+#endif
+
+		internal static readonly ConcurrentDictionary<Type, ConstructorInfo> constructors = new ConcurrentDictionary<Type, ConstructorInfo> ();
+		internal static readonly Dictionary<IntPtr, WeakReference> instances = new Dictionary<IntPtr, WeakReference> ();
+
+		internal static readonly ReaderWriterLockSlim instancesLock = new ReaderWriterLockSlim ();
+
+		/// <summary>
+		/// Retrieve the living instance if there is one, or null if not.
+		/// </summary>
+		/// <returns>The instance if it is alive, or null if there is none.</returns>
+		internal static bool GetInstance<TSkiaObject> (IntPtr handle, out TSkiaObject instance)
+			where TSkiaObject : SKObject
+		{
+			if (handle == IntPtr.Zero) {
+				instance = null;
+				return false;
+			}
+
+			instancesLock.EnterReadLock ();
+			try {
+				return GetInstanceNoLocks (handle, out instance);
+			} finally {
+				instancesLock.ExitReadLock ();
+			}
+		}
+
+		/// <summary>
+		/// Retrieve or create an instance for the native handle.
+		/// </summary>
+		/// <returns>The instance, or null if the handle was null.</returns>
+		internal static TSkiaObject GetObject<TSkiaObject, TSkiaImplementation> (IntPtr handle, bool owns = true, bool unrefExisting = true, bool refNew = false)
+			where TSkiaObject : SKObject
+			where TSkiaImplementation : SKObject, TSkiaObject
+		{
+			if (handle == IntPtr.Zero)
+				return null;
+
+			instancesLock.EnterUpgradeableReadLock ();
+			try {
+				if (GetInstanceNoLocks<TSkiaObject> (handle, out var instance)) {
+					// some object get automatically referenced on the native side,
+					// but managed code just has the same reference
+					if (unrefExisting && instance is ISKReferenceCounted refcnt) {
+#if THROW_OBJECT_EXCEPTIONS
+						if (refcnt.GetReferenceCount () == 1)
+							throw new InvalidOperationException (
+								$"About to unreference an object that has no references. " +
+								$"H: {handle.ToString ("x")} Type: {instance.GetType ()}");
+#endif
+						refcnt.SafeUnRef ();
+					}
+
+					return instance;
+				}
+
+				var type = typeof (TSkiaImplementation);
+				var constructor = constructors.GetOrAdd (type, t => GetConstructor (t));
+
+				// we don't need to go into a writable here as the object will do it in the Handle property
+				var obj = (TSkiaObject)constructor.Invoke (new object[] { handle, owns });
+				if (refNew && obj is ISKReferenceCounted toRef)
+					toRef.SafeRef ();
+				return obj;
+			} finally {
+				instancesLock.ExitUpgradeableReadLock ();
+			}
+
+			static ConstructorInfo GetConstructor (Type type)
+			{
+				var ctors = type.GetTypeInfo ().DeclaredConstructors;
+
+				foreach (var ctor in ctors) {
+					var param = ctor.GetParameters ();
+					if (param.Length == 2 && param[0].ParameterType == IntPtrType && param[1].ParameterType == BoolType)
+						return ctor;
+				}
+
+				throw new MissingMethodException ($"No constructor found for {type.FullName}.ctor(System.IntPtr, System.Boolean)");
+			}
+		}
+
+		/// <summary>
+		/// Retrieve the living instance if there is one, or null if not. This does not use locks.
+		/// </summary>
+		/// <returns>The instance if it is alive, or null if there is none.</returns>
+		private static bool GetInstanceNoLocks<TSkiaObject> (IntPtr handle, out TSkiaObject instance)
+			where TSkiaObject : SKObject
+		{
+			if (instances.TryGetValue (handle, out var weak) && weak.IsAlive) {
+				if (weak.Target is TSkiaObject match) {
+					if (!match.IsDisposed) {
+						instance = match;
+						return true;
+					}
+#if THROW_OBJECT_EXCEPTIONS
+				} else if (weak.Target is SKObject obj) {
+					if (!obj.IsDisposed && obj.OwnsHandle) {
+						throw new InvalidOperationException (
+							$"A managed object exists for the handle, but is not the expected type. " +
+							$"H: {handle.ToString ("x")} Type: ({obj.GetType ()}, {typeof (TSkiaObject)})");
+					}
+				} else if (weak.Target is object o) {
+					throw new InvalidOperationException (
+						$"An unknown object exists for the handle when trying to fetch an instance. " +
+						$"H: {handle.ToString ("x")} Type: ({o.GetType ()}, {typeof (TSkiaObject)})");
+#endif
+				}
+			}
+
+			instance = null;
+			return false;
+		}
+
+		/// <summary>
+		/// Registers the specified instance with the dictionary.
+		/// </summary>
+		internal static void RegisterHandle (IntPtr handle, SKObject instance)
+		{
+			if (handle == IntPtr.Zero || instance == null)
+				return;
+
+			SKObject objectToDispose = null;
+
+			instancesLock.EnterWriteLock ();
+			try {
+				if (instances.TryGetValue (handle, out var oldValue) && oldValue.Target is SKObject obj && !obj.IsDisposed) {
+#if THROW_OBJECT_EXCEPTIONS
+					if (obj.OwnsHandle) {
+						// a mostly recoverable error
+						// if there is a managed object, then maybe something happened and the native object is dead
+						throw new InvalidOperationException (
+							$"A managed object already exists for the specified native object. " +
+							$"H: {handle.ToString ("x")} Type: ({obj.GetType ()}, {instance.GetType ()})");
+					}
+#endif
+					// this means the ownership was handed off to a native object, so clean up the managed side
+					objectToDispose = obj;
+				}
+
+				instances[handle] = new WeakReference (instance);
+			} finally {
+				instancesLock.ExitWriteLock ();
+			}
+
+			// dispose the object we just replaced
+			objectToDispose?.DisposeInternal ();
+		}
+
+		/// <summary>
+		/// Removes the registered instance from the dictionary.
+		/// </summary>
+		internal static void DeregisterHandle (IntPtr handle, SKObject instance)
+		{
+			if (handle == IntPtr.Zero)
+				return;
+
+			instancesLock.EnterWriteLock ();
+			try {
+				var existed = instances.TryGetValue (handle, out var weak);
+				if (existed && (!weak.IsAlive || weak.Target == instance)) {
+					instances.Remove (handle);
+				} else {
+#if THROW_OBJECT_EXCEPTIONS
+					InvalidOperationException ex = null;
+					if (!existed) {
+						// the object may have been replaced
+
+						if (!instance.IsDisposed) {
+							// recoverable error
+							// there was no object there, but we are still alive
+							ex = new InvalidOperationException (
+								$"A managed object did not exist for the specified native object. " +
+								$"H: {handle.ToString ("x")} Type: {instance.GetType ()}");
+						}
+					} else if (weak.Target is SKObject o && o != instance) {
+						// there was an object in the dictionary, but it was NOT this object
+
+						if (!instance.IsDisposed) {
+							// recoverable error
+							// there was a new living object there, but we are still alive
+							ex = new InvalidOperationException (
+								$"Trying to remove a different object with the same native handle. " +
+								$"H: {handle.ToString ("x")} Type: ({o.GetType ()}, {instance.GetType ()})");
+						}
+					}
+					if (ex != null) {
+						if (instance.fromFinalizer)
+							exceptions.Add (ex);
+						else
+							throw ex;
+					}
+#endif
+				}
+			} finally {
+				instancesLock.ExitWriteLock ();
+			}
+		}
+	}
+}

binding/Binding/SKBitmap.cs

@@ -718,7 +718,10 @@ public bool PeekPixels (SKPixmap pixmap)
 			if (pixmap == null) {
 				throw new ArgumentNullException (nameof (pixmap));
 			}
-			return SkiaApi.sk_bitmap_peek_pixels (Handle, pixmap.Handle);
+			var result = SkiaApi.sk_bitmap_peek_pixels (Handle, pixmap.Handle);
+			if (result)
+				pixmap.pixelSource = this;
+			return result;
 		}
 
 		// Resize

binding/Binding/SKColorSpace.cs

@@ -164,7 +164,7 @@ public bool GetNumericalTransferFunction (out SKColorSpaceTransferFn fn)
 		// *XyzD50
 
 		public SKMatrix44 ToXyzD50 () =>
-			GetObject<SKMatrix44> (SkiaApi.sk_colorspace_as_to_xyzd50 (Handle), false);
+			OwnedBy (GetObject<SKMatrix44> (SkiaApi.sk_colorspace_as_to_xyzd50 (Handle), false), this);
 
 		public bool ToXyzD50 (SKMatrix44 toXyzD50)
 		{
@@ -175,7 +175,7 @@ public bool ToXyzD50 (SKMatrix44 toXyzD50)
 		}
 
 		public SKMatrix44 FromXyzD50 () =>
-			GetObject<SKMatrix44> (SkiaApi.sk_colorspace_as_from_xyzd50 (Handle), false);
+			OwnedBy (GetObject<SKMatrix44> (SkiaApi.sk_colorspace_as_from_xyzd50 (Handle), false), this);
 
 		//
 

binding/Binding/SKData.cs

@@ -251,6 +251,7 @@ public void SaveTo (Stream target)
 			} finally {
 				ArrayPool<byte>.Shared.Return (buffer);
 			}
+			GC.KeepAlive (this);
 		}
 
 		//

binding/Binding/SKDocument.cs

@@ -21,10 +21,10 @@ public void Abort () =>
 			SkiaApi.sk_document_abort (Handle);
 
 		public SKCanvas BeginPage (float width, float height) =>
-			GetObject<SKCanvas> (SkiaApi.sk_document_begin_page (Handle, width, height, null), false);
+			OwnedBy (GetObject<SKCanvas> (SkiaApi.sk_document_begin_page (Handle, width, height, null), false), this);
 
 		public SKCanvas BeginPage (float width, float height, SKRect content) =>
-			GetObject<SKCanvas> (SkiaApi.sk_document_begin_page (Handle, width, height, &content), false);
+			OwnedBy (GetObject<SKCanvas> (SkiaApi.sk_document_begin_page (Handle, width, height, &content), false), this);
 
 		public void EndPage () =>
 			SkiaApi.sk_document_end_page (Handle);

binding/Binding/SKFontStyle.cs

@@ -4,6 +4,19 @@ namespace SkiaSharp
 {
 	public class SKFontStyle : SKObject
 	{
+		private static readonly Lazy<SKFontStyle> normal;
+		private static readonly Lazy<SKFontStyle> bold;
+		private static readonly Lazy<SKFontStyle> italic;
+		private static readonly Lazy<SKFontStyle> boldItalic;
+
+		static SKFontStyle()
+		{
+			normal = new Lazy<SKFontStyle> (() => new SKFontStyleStatic (SKFontStyleWeight.Normal, SKFontStyleWidth.Normal, SKFontStyleSlant.Upright));
+			bold = new Lazy<SKFontStyle> (() => new SKFontStyleStatic (SKFontStyleWeight.Bold, SKFontStyleWidth.Normal, SKFontStyleSlant.Upright));
+			italic = new Lazy<SKFontStyle> (() => new SKFontStyleStatic (SKFontStyleWeight.Normal, SKFontStyleWidth.Normal, SKFontStyleSlant.Italic));
+			boldItalic = new Lazy<SKFontStyle> (() => new SKFontStyleStatic (SKFontStyleWeight.Bold, SKFontStyleWidth.Normal, SKFontStyleSlant.Italic));
+		}
+
 		[Preserve]
 		internal SKFontStyle (IntPtr handle, bool owns)
 			: base (handle, owns)
@@ -37,12 +50,26 @@ protected override void DisposeNative () =>
 
 		public SKFontStyleSlant Slant => SkiaApi.sk_fontstyle_get_slant (Handle);
 
-		public static SKFontStyle Normal => new SKFontStyle (SKFontStyleWeight.Normal, SKFontStyleWidth.Normal, SKFontStyleSlant.Upright);
+		public static SKFontStyle Normal => normal.Value;
 
-		public static SKFontStyle Bold => new SKFontStyle (SKFontStyleWeight.Bold, SKFontStyleWidth.Normal, SKFontStyleSlant.Upright);
+		public static SKFontStyle Bold => bold.Value;
 
-		public static SKFontStyle Italic => new SKFontStyle (SKFontStyleWeight.Normal, SKFontStyleWidth.Normal, SKFontStyleSlant.Italic);
+		public static SKFontStyle Italic => italic.Value;
 
-		public static SKFontStyle BoldItalic => new SKFontStyle (SKFontStyleWeight.Bold, SKFontStyleWidth.Normal, SKFontStyleSlant.Italic);
+		public static SKFontStyle BoldItalic => boldItalic.Value;
+
+		private sealed class SKFontStyleStatic : SKFontStyle
+		{
+			internal SKFontStyleStatic (SKFontStyleWeight weight, SKFontStyleWidth width, SKFontStyleSlant slant)
+				: base (weight, width, slant)
+			{
+				IgnorePublicDispose = true;
+			}
+
+			protected override void Dispose (bool disposing)
+			{
+				// do not dispose
+			}
+		}
 	}
 }

binding/Binding/SKImage.cs

@@ -529,7 +529,11 @@ public bool PeekPixels (SKPixmap pixmap)
 		{
 			if (pixmap == null)
 				throw new ArgumentNullException (nameof (pixmap));
-			return SkiaApi.sk_image_peek_pixels (Handle, pixmap.Handle);
+
+			var result = SkiaApi.sk_image_peek_pixels (Handle, pixmap.Handle);
+			if (result)
+				pixmap.pixelSource = this;
+			return result;
 		}
 
 		public SKPixmap PeekPixels ()