Xamarin.Forms
Xamarin.Forms copied to clipboard
xamarin
Set CALayer and UIView on main thread
Description of Change
This fixes cases where CALayer and UIView are sometimes modified by background threads. The previous code was inconsistent, in one place ensuring that CALayer.Transform was called on the main thread and in another place not. Now, modification is always on the main thread.
I made the change solely based on examining the existing code. I have not tested or even compiled the change.
Issues Resolved
- fixes #9469
Platforms Affected
- iOS
PR Checklist
- [ ] Targets the correct branch [It probably doesn't. I didn't see the note about targeting latest stable until I had already made the commit from master in GitHub. It would be great if someone could move the change to the proper branch.]
- [ ] Tests are passing (or failures are unrelated)
Hi, error #9469 probably depends on this line of code, because it returns false when in place of true.
to fix this issue you can change line like this:
var thread = Application.Current?.OnThisPlatform()?.GetHandleControlUpdatesOnMainThread() == false;
@workgroupengineering I wondered about that line. I don't understand it well. The thread variable determines whether the update is invoked with CADisplayLinkTicker, which lacks a summary comment, but seems to be intended to move operations off the main thread onto a background thread. If that's the case, the flow is to start on the main thread, switch to a background thread to calculate the layout, and finally switch back to the main thread to apply the update. However, the final switch to a main thread is not always done, which is what my PR fixes.
If my understanding is correct, calling SetHandleControlUpdatesOnMainThread(true) would avoid the crash in my app. I've put that change in place and am monitoring the crash telemetry results.
On the topic of checking GetHandleControlUpdatesOnMainThread, I agree that !boundsChanged && !caLayer.Frame.IsEmpty don't seem to add any value, especially in the context of the modified code. I also wondered about the null checking on that line. Is it possible for this code be called before Application.Current is set? If not, assuming the logic isn't reversed, the code should be more simply:
thread = !Application.Current.OnThisPlatform().GetHandleControlUpdatesOnMainThread();
Some better variable naming would be helpful, too. It looks like thread could be better named shouldPlanLayoutOnBackgroundThread.
One other concern is reading from CALayer and UIView. My change cleans up the writes, but there are a few reads that can happen from background threads. Are they guaranteed safe?
Hi @breyed, did you find any more crashes of this type in your telemetry after applying this fix? I'm thinking about merging your changes onto our fork too...
@bruzkovsky We didn't create a custom build of Xamarin.Forms with the fix. We don't have any recent crashes from this issue. However, I don't remember if it's because we designed around the issue or if it just stopped reproducing on its own.
Hey @breyed I can confirm we're still getting this crash intermittently on iOS 14.x, we're currently on Xamarin Forms 5.0.0.2012.
I've attached the crash log from AppCenter.
report-2517701916350009999-48e54047-9921-4ebb-b29c-4adafa3baf15.txt
[EDIT] And to be clear, we're seeing this on an iPad Mini 5
I think at this point it's safe to say that we won't be taking this for Xamarin.Forms anymore. Thank you all so much for your time and effort on this. It hasn't gone unnoticed!
