Windows-XAML / Template10

Making Windows 10 apps great again
Apache License 2.0
1.41k stars 389 forks source link

Weird System.AccessViolationException related to FlipView and OnNavigatedToAsync #704

Closed DanTheManSWE closed 8 years ago

DanTheManSWE commented 8 years ago

NuGet Version 1.1.5 When I try to set a field that is bound to SelectedIndex of my FlipView Control from OnNavigatedToAsync I get a System.AccessViolationException in Template10.dll. I can't seem to find the NuGet branch for 1.1.5 so I haven't investigated any further.

The first time I navigate to the page it works fine, but if I go back to the mainpage, and then navigate to the page again I will get an exception if the value is different from the first time I navigated there.

However, if I place the same code in the constructor of the ViewModel it works fine, or if I use a ListView control instead of FlipView.

So, the issue is probably not related to Template10, but the FlipView control itself, but I wanted to know if anyone else have experienced the same?

StackTrace: at System.ComponentModel.PropertyChangedEventHandler.Invoke(Object sender, PropertyChangedEventArgs e) at Template10.Mvvm.BindableBase.RaisePropertyChanged(String propertyName) at Template10.Mvvm.BindableBase.Set[T](T& storage, T value, String propertyName) at IronPlanner.ViewModels.StatsPageViewModel.set_CharacterIndex(Int32 value) at IronPlanner.ViewModels.StatsPageViewModel.<OnNavigatedToAsync>d__31.MoveNext() at System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.InvokeMoveNext(Object stateMachine) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) EDIT Seems like this still happens even when setting the field from the constructor, although it's very rare then.

JustinXinLiu commented 8 years ago

I would post this with code on StackOverflow.

DanTheManSWE commented 8 years ago

Yeah, I need to reproduce it without Template10 first, just thought I'd mention it here if anyone had similar experiences.

ghost commented 8 years ago

I have the same problem, haven't fix yet

JerryNixon commented 8 years ago

Please let me know if you can repro this.

Opiumtm commented 8 years ago

@DanTheManSWE

I think there is some async related problem. AccessViolationException mean there is a threading issue, FlipView dependency property is accessed from not an UI thread. Similar issue was with NavButtonBehavior when under some circumstances on ARM device when compiled with .NET Native toolchain exception was raised and it was fixed by explicitly dispatching a call to UI thread. So, you should not rely on a fact that your async method is called from UI thread and it seems to be a platform and toolchain dependent difference in behavior. As I can assume, there is no guarantee that Task async parallel framework schedule all code on UI thread especially when you are using async void language construction instead of async Task. In general you should avoid use async void at all, because there are also exception handling issues with this pattern - exceptions from this method are propagated to top application level and could crash application as "unhandled". Personally instead of plain async void construction I prefer to excplicitly dispatch async function calls to core dispatcher with exception handler (I use Template10's DispatcherWrapper) to guarantee correct work on UI thread. In general async void method pattern have too many issues and should be avoided.

Opiumtm commented 8 years ago

@JerryNixon Need check if async void method pattern is used in OnNavigatedToAsync related implementation. If there is a case, there will be a platform dependent thread issues. Some async related threading bugs are just not reproducing on x86 desktop devices but show on ARM.

Opiumtm commented 8 years ago

@JerryNixon Just looked into the code. Not sure if

                await WindowWrapper.Current().Dispatcher.DispatchAsync(async () =>
                {
                    try
                    {
                        await NavigateToAsync(e.NavigationMode, parameter, FrameFacadeInternal.Frame.Content);
                    }
                    catch (Exception ex)
                    {
                        DebugWrite($"DispatchAsync/NavigateToAsync {ex.Message}");
                        throw;
                    }
                }, 1);

lambda function is compled as async Task or async void, as there are two overloaded methods of DispatcherWrapper which take both Action and Func<Task> as argument. In case of lamda function it can be async void instead of async Task. It will be lot better to use delegate constructor with Func<Task> type instead of implicit constructor with plain lambda syntax.

DanTheManSWE commented 8 years ago

Thank you for the input, the issue may be in one of my own async methods.

Skickat från min Windows Phone

----- Ursprungligt meddelande ----- Från: "Eugeny Ipatov" notifications@github.com Skickat: ‎2016-‎03-‎01 15:42 Till: "Windows-XAML/Template10" Template10@noreply.github.com Kopia: "DanTheManSWE" dan.o.eriksson@gmail.com Ämne: Re: [Template10] Weird System.AccessViolationException related toFlipView and OnNavigatedToAsync (#704)

@DanTheManSWE I think there is some async related problem. AccessViolationException mean there is a threading issue, FlipView dependency property is called from a not UI thread. Similar issue was with NavButtonBehavior when under some circumstandes on ARM device when compiled on .NET Native toolchain exception was raised and it was fixed by explicitly dispatching a call to UI thread. So, you should not rely on a fact that your async method is called from UI thread and it seems to be platform and toolchain dependent difference in behavior. As I can assume, there is no guarantee that Task async parallel framework schedule all code on UI thread especially when you are using async void language construction instead of async Task. In general you should avoid use async void at all, because there is also an exception handling issues with this pattern - exceptions from this method are propagated to top a pplication level and could crash application as unhandled exception. Personally instead of plain async void construction I prefer to excplicitly dispatch async function call to dispatcher wrapped in exception handler to guarantee correct work. In general async void method signature work with too many issues and should be avoided. — Reply to this email directly or view it on GitHub.

JerryNixon commented 8 years ago

@Opiumtm the c# syntax (async () => { }) translates to async Task Method() { }.

@DanTheManSWE, this issue is too vague for me to chase. If you get more information on how to repro. it, open a new issue and I will look deeper into it. k?

DanTheManSWE commented 8 years ago

Of course, as I said, it's probably related to FlipView in combination with async calls since I can't reproduce it at all with a ListView, I'm also gonna look in to using a dispatcher, learning new stuff every day ;)

jarhead19591963 commented 8 years ago

I had this issue with flipview as well and resolved it by uninstalling and reinstalling the intense toolkit from first floor software there is also an nuget update for netcore.universalwindowsplatform version 5.2.2