From 6436bb348141ced2838514aa3f5c5515468df017 Mon Sep 17 00:00:00 2001 From: Jose Medrano Date: Tue, 2 Aug 2022 18:10:59 +0200 Subject: [HistoryLayer] On viewmodel changes we ensure our viewmodel is not null and set default color (#821) Fix [AB#1569130](https://devdiv.visualstudio.com/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_workitems/edit/1569130) Due to the issue https://github.com/xamarin/xamarin-macios/issues/15600, when a CGColor is allocated and its native object about to be passed to PInvoke when setting CALayout.BackgroundColor, it's possible for it be GC'ed, which CFReleases the CGColor, before the PInvoke completes and BackgroundColor gets a chance to CFRetain it. In that case, the BackgroundColor can point to a deleted object. Until there's a real fix for that race condition with https://github.com/xamarin/xamarin-macios/issues/15600 fixed, we workaround it here by doing a GC.KeepAlive ensuring that the object lives until the PInvoke completes. Also, in some scenarios looks like ViewModel is not yet set in OnViewModelChanged event, which could cause a NRE. We now protect against that too. [CleanShot 2022-07-21 at 17 38 24@2x] (https://github.com/xamarin/Xamarin.PropertyEditing/blob/dev/netonjm/fix-1569130/Xamarin.PropertyEditing.Mac/Controls/Custom/SolidColorBrushEditorViewController.cs#L33-L47) Co-authored-by: Bret Johnson --- .../Controls/Custom/HistoryLayer.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Xamarin.PropertyEditing.Mac/Controls/Custom/HistoryLayer.cs b/Xamarin.PropertyEditing.Mac/Controls/Custom/HistoryLayer.cs index 9f90d61..d4ec241 100644 --- a/Xamarin.PropertyEditing.Mac/Controls/Custom/HistoryLayer.cs +++ b/Xamarin.PropertyEditing.Mac/Controls/Custom/HistoryLayer.cs @@ -3,6 +3,7 @@ using AppKit; using CoreAnimation; using CoreGraphics; using ObjCRuntime; +using Xamarin.PropertyEditing.Drawing; namespace Xamarin.PropertyEditing.Mac { @@ -77,9 +78,20 @@ namespace Xamarin.PropertyEditing.Mac public override void UpdateFromModel (EditorInteraction interaction) { LayoutIfNeeded (); - current.BackgroundColor = interaction.Color.ToCGColor (); - previous.BackgroundColor = interaction.ViewModel.InitialColor.ToCGColor (); - last.BackgroundColor = interaction.LastColor.ToCGColor (); + SetCALayerBackgroundColor (current, interaction.Color); + //there are some scenarios where viewmodel could be null + if (interaction.ViewModel != null) + SetCALayerBackgroundColor (previous, interaction.ViewModel.InitialColor); + SetCALayerBackgroundColor (last, interaction.LastColor); + } + + static void SetCALayerBackgroundColor(CALayer layer, CommonColor color) + { + // We need to use GC.KeepAlive to ensure that the CGColor doesn't get garbage collected (and thus the + // native object Released) before the BackgroundColor assignment is complete + CGColor cgColor = color.ToCGColor (); + layer.BackgroundColor = cgColor; + GC.KeepAlive (cgColor); } public override void UpdateFromLocation (EditorInteraction interaction, CGPoint location) -- cgit v1.2.3