xamarin / Xamarin.PropertyEditing

MIT License
24 stars 16 forks source link

[HistoryLayer] On viewmodel changes we ensure our viewmodel is not null and set default color #821

Closed netonjm closed 2 years ago

netonjm commented 2 years ago

Fix AB#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)

BretJohnson commented 2 years ago

@Therzok I updated the PR comment to explain the issue. I'll do a squash when merging so it becomes the commit comment too.