xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.62k stars 1.87k forks source link

[Bug] Application crashes on Android when power saving is enabled #8382

Open wojciech-kulik opened 4 years ago

wojciech-kulik commented 4 years ago

Description

For the last two weeks, I was trying to find a root cause for an issue that freezes my application on a splash screen on some devices. It was possible to reproduce only on Galaxy S8 and S9 and only on some devices.

After two weeks and countless hours, I finally managed to trace the root cause. This issue happens when battery power saving mode is enabled (on Galaxy it is specifically medium power consumption mode) and animation is invoked in a loop like this:

async Task RotateElement(VisualElement element)
{
    while (true)
    {
        await element.RotateTo(360, 1200);
        element.Rotation = 0;
    }
}

After some more investigation, I found out that it was broken since Xamarin.Forms 3.1.0.637273. Using Xamarin.Forms 3.1.0.583944 it works properly. Following this lead, I found out that in the broken version there was a fix for issue #4176 and I think it causes this behavior. Probably because of await operator which doesn't work anymore as expected (because of the fix which runs code on a Looper threads) and invoking two times RotateTo causes freeze.

Adding permission android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS does not help.

Steps to Reproduce

  1. Switch Android device to battery saving mode (medium power consumption option).
  2. Open the application and run animation.

Expected Behavior

The application should not crash/freeze. I think that "medium power consumption" should not turn off animations either.

Actual Behavior

Application freezes, stops responding and eventually crashes.

Basic Information

Reproduction Link

https://github.com/wojciech-kulik/XamarinPowerSavingCrash

There is nothing special in application logs during a freeze.

Workaround

I found out that it is possible to provide some workaround by adding await Task.Delay(1) at the end of loop, but I don't know how reliable it is.

bcaceiro commented 4 years ago

I've ran into this issue as well in the past. It is all related to the battery settings, and any type of animations simply gets ignored. The solution I found out, was to surround the animations in a try catch, and in the catch, simple change the values to the final of the animation. Though this should be the default behaviour from the platform.

hartez commented 4 years ago
async Task RotateElement(VisualElement element)
{
    while (true)
    {
        await element.RotateTo(360, 1200);
        element.Rotation = 0;
    }
}

@wojciech-kulik How are you invoking this method? Power management optimizations aside, this looks like it would just animate your element for the lifetime of the application, even if it's not on-screen.

wojciech-kulik commented 4 years ago

@hartez this is a simplification for better understanding. In my case, I have a flag that stops the animation after data is loaded.

In the meantime, I discovered the exact reason for this issue. In power-saving mode, this line await element.RotateTo(360, 1200) finishes immediately which causes loop with no delay which overfloods main thread - freezes the application.

The problem is that there is an assumption that RotateTo will not exit earlier than after 1200ms, which is not true in power-saving mode.

I think this method should be corrected. It worked as expected in Xamarin.Forms 3.1.0.583944. This animation should also work in power-saving mode, because native animations do work.

To fix this issue and have animation in a power-saving mode, I prepared workaround using custom renderer:

using System;
using System.ComponentModel;
using Android.Content;
using Android.Views.Animations;
using Project.Controls;
using Project.Droid.CustomRenderers;
using Xamarin.Forms;
using Xamarin.Forms.Platform.Android;

[assembly: ExportRenderer(typeof(RefreshIcon), typeof(RefreshIconRenderer))]
namespace Project.Droid.CustomRenderers
{
    public class RefreshIconRenderer : ImageButtonRenderer
    {
        private RotateAnimation rotateAnimation;

        public RefreshIconRenderer(Context context) : base(context) { }

        protected override async void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            base.OnElementPropertyChanged(sender, e);

            if (e.PropertyName == RefreshIcon.IsRefreshingProperty.PropertyName)
            {
                var isRefreshing = (Element as RefreshIcon)?.IsRefreshing ?? false;
                this.Enabled = !isRefreshing;

                if (isRefreshing)
                {
                    this.rotateAnimation = new RotateAnimation(0.0f, 360.0f, Dimension.RelativeToSelf, 0.5f, Dimension.RelativeToSelf, 0.5f)
                    {
                        Duration = 1200,
                        RepeatMode = RepeatMode.Restart,
                        RepeatCount = Android.Views.Animations.Animation.Infinite,
                        FillAfter = false,
                        Interpolator = new LinearInterpolator()
                    };
                    this.StartAnimation(this.rotateAnimation);
                }
                else if (this.rotateAnimation != null)
                {
                    this.rotateAnimation.RepeatCount = 0;
                }
            }
        }
    }
}
hartez commented 4 years ago

In power saving mode on Android, the animation ticker does not fire at all (which is an OS thing; we don't (and shouldn't) have control over that). See ValueAnimator for more info. So animations simply will not run when power saving mode is active.

To address this, Forms watches for power saving mode and immediately moves all animations to their end state and finishes their Tasks. That's why your await is returning immediately. If we did not do that, any animations that attempted to run while the device was in power saving mode would hang; the awaited code would never return.

Your original code (as posted) makes the (incorrect) assumption that it will be giving up control of the main thread in between ValueAnimator updates, allowing the rest of your application to run. Since the awaited code returns immediately, there's no opportunity for any other work to be done.

PureWeen commented 4 years ago

Similar conversation here https://github.com/xamarin/Xamarin.Forms/issues/7500#issuecomment-538601670

wojciech-kulik commented 4 years ago

@hartez it's not true about animations in power-saving mode. They work properly in power-saving mode, otherwise, user experience would be quite poor if all animations would be disabled. Check out my workaround with the custom renderer. It uses native animation and it works in power-saving mode.

I can agree that returning immediately when an animation is not triggered makes sense. However, I think it will lead many developers to issue similar to mine which takes weeks to debug. It's enough to run a loop like mine for about one second to hang the whole application.

Google for "Xamarin.Forms infinite animation", all posts recommend using while loop and probably many people use this way to provide such animation which means that many people have this issue and they don't know about it or are not able to fix it. It freezes main thread, therefore there is no crash, no crash report in crashlytics and nothing meaningful in system logs, also it's super hard to figure out that power-saving mode is the root cause here.

I recommend the same solutions as @BioTurboNick in #7500 to return at least Task.Delay instead of instant return. However, the real solution to me would be moving from ValueAnimator to RotateAnimation, this way animation would also work in power-saving mode.

Regarding the link that you provided to ValueAnimator. Please see that ValueAnimator inherits from Animator which is probably dedicated for more resource-consuming animations. Regular UI animations like RotateAnimation inherit from Animation, they don't have property areAnimatorsEnabled and they all work in power-saving mode. Also Animation is in namespace related to UI android.view and Animator is just in android which may suggest its intended usage. However, as usual, Android documentation is quite poor and there is no explanation about that.

Anyway, I've never observed apps that stop basic small animations in power-saving mode. Imagine activity indicator not spinning in power-saving mode. The user experience would be terrible.

hartez commented 4 years ago

@wojciech-kulik I think we've got a plan to address this, see https://github.com/xamarin/Xamarin.Forms/issues/7500#issuecomment-552153036

wojciech-kulik commented 4 years ago

@hartez Great to hear! I'm looking forward to the updated version :)

AlonRom commented 4 years ago

@wojciech-kulik I have the same issue, do you have a full example using this renderer? or a better solution? It happened to me 1 time on iPhone device with "Low Battery Mode" but I can reproduce in Samsung devices all the time.

wojciech-kulik commented 4 years ago

@AlonRom hi, here is code causing the issue in my case: https://github.com/xamarin/Xamarin.Forms/issues/8382#issue-517812411

and here is a solution I created: https://github.com/xamarin/Xamarin.Forms/issues/8382#issuecomment-549959729

It uses built-in animation which repeats instead of animation in a loop which freezes main thread in battery saving mode.

AlonRom commented 4 years ago

@wojciech-kulik thanks, do I have to use a renderer, can't we do at core level? Your solution is only for Android.

wojciech-kulik commented 4 years ago

This bug is on Android, so you don't need a solution for iOS. I don't know what is "core level". Everything is on this ticket. So read it carefully and figure out if this applies to your problem.

AlonRom commented 4 years ago

Actually it happened to us on iOS as well when the device was on "Low Battery Mode", but I agree it's not very common. Thanks anyway.