Xamarin.forms: [Bug] [Android] Async animations can cause hang if animations are disabled

[BioTurboNick]

Description

One simple way to do a looped animation is to do an async void method containing an infinite loop. Normally, this works fine because the async system allows other code to execute in the meantime.

However, if animations become disabled for some reason, such as due to power saving mode or the accessibility setting, the async calls return immediately, causing the application to hang.

Steps to Reproduce

1. Write an async void method with an infinite loop (while (true)) containing simple animations (e.g. await view.ScaleTo(0.8, 500);)
2. Run the app to confirm the animation works and the app doesn't hang.
3. Turn off animations in Android settings, or by turning on a power savings mode.
4. Run the app again.

Expected Behavior

The animation should run and the app shouldn't hang. Suggestion: the animation should return Task.Delay() of the same duration instead of immediately returning.

Actual Behavior

The app hangs with an infinite loop.

Basic Information

• Version with issue: 4.2.0.778463
• Last known good version: n/a
• IDE: Visual Studio 2019 16.2.5
• Platform Target Frameworks:
  
  
  
  • Android: 9.0
  
  
• Android Support Library Version:
• Nuget Packages:
• Affected Devices: Samsung Galaxy S9+

Reproduction Link

Later, if really needed.

AsyncAnimationHang.zip

[BioTurboNick]

Just took a look at the code and see it might be hard to try detecting the state of the device?

If so, an alternative solution could be to have ScaleTo and related methods spin up both a Task.Delay and the animation task and return Task.WhenAll();

https://github.com/xamarin/Xamarin.Forms/blob/bd31e1e9fc8b2f9ad94cc99e0c7ab058174821f3/Xamarin.Forms.Core/ViewExtensions.cs

e.g. AnimateTo() could return Task.WhenAll([tcs.Task, Task.Delay(length)]);

[hartez]

Reproduction Link
Later, if really needed.

Really needed.

[BioTurboNick]

Attached to OP.

[hartez]

I added a button to your repro application which displays an alert, then ran it with animations off on my device. As far as I can tell, the application isn't hanging; I can still tap the button and display the alert. Am I misunderstanding the issue?

AsyncAnimationHang_WithButton.zip

[BioTurboNick]

Thanks for checking. I see no difference with your code, but I do see that the two places in Settings to turn off animations (Visibility enhancements, Developer options) do not cause the bug. It's only caused by turning on the Medium Power Saving mode.

When I am in "Optimized" (normal) power mode, the button renders/functions and the animation runs. When I use those two settings, the button renders/functions and the animation is frozen.

In "Medium Power Saving" mode, when I run the app nothing renders. It's just a blank white screen with blue status bar. Accompanying this is a ton of GC debug messages in the debug output that don't happen when it is running normally.

e.g.

09-24 19:14:51.260 D/Mono    (24818): GC_TAR_BRIDGE bridges 12 objects 13 opaque 0 colors 12 colors-bridged 12 colors-visible 12 xref 0 cache-hit 0 cache-semihit 0 cache-miss 0 setup 0.05ms tarjan 0.01ms scc-setup 0.03ms gather-xref 0.00ms xref-setup 0.00ms cleanup 0.01ms
09-24 19:14:51.260 D/Mono    (24818): GC_BRIDGE: Complete, was running for 9.37ms
09-24 19:14:51.260 D/Mono    (24818): GC_MAJOR_CONCURRENT_FINISH: (finishing) time 97.10ms, stw 9.75ms los size: 1024K in use: 54K
09-24 19:14:51.260 D/Mono    (24818): GC_MAJOR_SWEEP: major size: 2608K in use: 860K
09-24 19:14:51.364 D/Mono    (24818): GC_TAR_BRIDGE bridges 0 objects 0 opaque 0 colors 0 colors-bridged 0 colors-visible 12 xref 0 cache-hit 0 cache-semihit 0 cache-miss 0 setup 0.05ms tarjan 0.01ms scc-setup 0.03ms gather-xref 0.00ms xref-setup 0.00ms cleanup 0.00ms
09-24 19:14:51.364 D/Mono    (24818): GC_BRIDGE: Complete, was running for 0.15ms
09-24 19:14:51.364 D/Mono    (24818): GC_MINOR: (Nursery full) time 7.21ms, stw 8.51ms promoted 304K major size: 2624K in use: 1178K los size: 1024K in use: 54K
09-24 19:14:51.457 D/Mono    (24818): GC_TAR_BRIDGE bridges 0 objects 0 opaque 0 colors 0 colors-bridged 0 colors-visible 12 xref 0 cache-hit 0 cache-semihit 0 cache-miss 0 setup 0.05ms tarjan 0.01ms scc-setup 0.03ms gather-xref 0.00ms xref-setup 0.00ms cleanup 0.00ms
09-24 19:14:51.457 D/Mono    (24818): GC_BRIDGE: Complete, was running for 0.17ms

There are multiple settings associated with "Medium Power Saving" mode that can be optionally removed, but none of them appear to change the behavior.

[PureWeen]

I feel like the failure here is a way to articulate that an animation has failed or that they are disabled.

Animations have a return value that I don't quite understand. Maybe @hartez can expand on it a bit :-) It seems to return false if the animation succeeded and then true if it's aborted.

If those assignments are correct than I feel like if Animations are disabled they should return true

Suggestion: the animation should return Task.Delay() of the same duration instead of immediately returning.

I don't think giving the illusion of a playing animation is the correct approach. If the code creates fake time for a non playing animation then the user will just see nothing happening and the developer will think something is happening. If there's an infinite loop of animation I would think it'd go something like this

C# var keepRunning = true; while (keepRunning) { keepRunning = !(await label.ScaleTo(2, 500)); keepRunning = !(await label.ScaleTo(0.5, 500)); }

so that you just exit the loop if animations aren't working or you have some event you can tap into indicating animations have come back or are disabled.

Or don't use an infinite loop and use this construct

https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/animation/custom

which is more tailored for repeating animations

By using a while loop I think you run the risk of blocking the UI Thread where as if you use the custom approach above then our animation code handles the Queueing of animation in such a way that's more app friendly

[BioTurboNick]

Thanks @PureWeen . I did figure out how to use the Animation classes eventually, and those don't have this issue as you suggested. Took a bit, but I think a lot of that was compounded by the animations-off aspect of the power mode.

The return value is documented here in the first big paragraph: https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/animation/simple but the return value appears not to be documented with the functions themselves.

I suppose a follow-up question would be: what cancels an animation, vs. stopping it from running? That is, why does turning animations globally off in Settings not cause this hang?

Directing people away from doing animations in loops or to check the value seems sensible (I'll verify if it works as you suspect later), but it seems like a sneaky source of hangs that could be hard to diagnose.

[PureWeen]

The return value is documented here in the first big paragraph:

There it is! I still feel like it's inverted but can't really do anything about that

I suppose a follow-up question would be: what cancels an animation, vs. stopping it from running?

Cancelling happens if you start an animation then startup another one before the previous one has finished. If you trigger an animation from a button click and then keep clicking the button. If the previous ScaleTo hasn't finished and you trigger another one it'll abort the first one.

With stopping it from running the task just completes it immediately if Ticker.Default.SystemEnabled is set to false. For this case I feel like it should return true instead of false because it's technically aborting the animation before it starts

[BioTurboNick]

Oh no! Checking for the return value doesn't change anything. The loop runs, blocking the UI thread. The debugger lets me break inside the loop like nothing is wrong, and the ScaleTo functions return "false" each time.

For completeness, I went back to the normal power mode and then disabled animations via Settings, to see how the ScaleTo lines behaved. And... it does something different. It does that weird async thing where it never returns at all.

[PureWeen]

Oh no! Checking for the return value doesn't change anything. The loop runs, blocking the UI thread. The debugger lets me break inside the loop like nothing is wrong, and the ScaleTo functions return "false" each time.

Yea that's the point I'm making here

With stopping it from running the task just completes it immediately if Ticker.Default.SystemEnabled is set to false. For this case I feel like it should return true instead of false because it's technically aborting the animation before it starts

This is a bug IMO. It should return true if animations are disabled and never running and then having a hook somewhere indicating animations have been disabled/enabled would also be of use

[BioTurboNick]

I misunderstood at first, thank you.

Also, turns out the discrepancy between the two experiences may have been due to a bug in 4.1? I thought I had been using 4.2 in the test project, but going to 4.2 made them identical: immediate return => infinite loop. I apologize for the noise there.

One argument in favor of having a Task.Delay of ~ the same length: When I add it manually, it provides 2 benefits: prevent the UI thread from being blocked, and allows the object to cycle between end points for each component animation.

That is, when animations are off, with the repro sample, you would see the label jump between large and small every half second. That could arguably be a better fallback than an infinite loop or no animation at all; at least the end user would still be able to use the app, and could potentially report the animation as faulty. If a dev still wanted to check for success, they could do so for a better experience.

Anyway, my 2 cents. Thanks again!

[hartez]

Cancelling happens if you start an animation then startup another one before the previous one has finished. If you trigger an animation from a button click and then keep clicking the button. If the previous _ScaleTo_ hasn't finished and you trigger another one it'll abort the first one.

It also returns true (cancelled) if you use the AbortAnimation method.

[hartez]

Okay, I can see a few issues here.

I think @pureween is right, the animation extension methods should return true (canceled) when they are forced to finish immediately because animations are disabled. That will allow the infinite-loopers a way out of their loops. I'm 99% sure we can do this, because I believe the AbortAnimation method also forces the animation to its final state, so everything will remain consistent. (I'll verify this tomorrow; if that's not the case, this becomes more difficult.)

(Unfortunately there's nothing we can do about the confusing return values for those methods; I don't know why they return true for cancellation, but modifying that now would be a huge breaking change.)

I also agree that we should have some way for folks to determine whether animations are currently enabled. Technically, we already do (Ticker.Default.SystemEnabled), but it's hidden and non-intuitive. So I propose something like static bool AnimationsEnabled {get;} on the Animation class.

Finally, after reviewing the code that watches for power saving on Android, I can see a few issues (and why we have all this confusion around Samsung devices). The code is assuming that activating _any_ power-saving mode means that animations will be disabled. This was accurate once upon a time, because there weren't multiple power saving modes. But Samsung devices have multiple levels of power saving, and not all of them disable animation. So the code needs to be updated to re-check the state of animations when the power saving mode changes, and not assume animations are off. And we need to update the animation check to use AreAnimationsEnabled() for later APIs. I think making those changes will address most of the confusion and issue we're seeing here.