Windows-XAML / Template10

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

[Mobile] ArgumentException when Window.Current.Content is updated multiple times in T10 1.1.8 #745

Closed callummoffat closed 8 years ago

callummoffat commented 8 years ago

I've discovered a bug where setting Window.Current.Content multiple times in recent versions of Template 10 causes the app to crash on Windows 10 Mobile. There are several workarounds, but I have uncovered several cases where the crash is unavoidable.

This bug is, I believe, urgent. @Windows-XAML/community, does anyone have any ideas on how to remedy this?

mvermef commented 8 years ago

You should be able to trap or configure azure to authenticate with a form of your choice, I had issues with changing the current window content, and actually scrapped that authentication method using content switching, pretty much a pain in the ass.

callummoffat commented 8 years ago

I'll try doing that. Might look better as well. Do you have any advice as to how to manage that?

mvermef commented 8 years ago

Are you using the Providers available or are you using a custom backend to authenticate with Azure?

callummoffat commented 8 years ago

Also, this issue doesn't just affect Azure Mobile Apps. It affects any code that changes Window.Current.Content, which is why it's so urgent. Not only does this bug affect Template 10 out of the box (Hamburger template), it also affects any code, internal and external, that updates Window.Current.Content.

Internal code enables workarounds in most cases, but if the offending code is in an external library, there is no workaround for this issue. It makes using Template 10 more difficult, and that isn't good, considering it's designed to make a developer's life easier.

callummoffat commented 8 years ago

I'm using the available providers currently. It's less work, and I'm lazy. :-)

callummoffat commented 8 years ago

@JerryNixon, do you have any ideas about this issue? I need a fix as soon as possible.

bc3tech commented 8 years ago

@callummoffat the weird this is it doesn't affect anything that sets Window.Current.Content. If you crack open Minimal and set the SplashFactory which, if you look internally in to T10, sets the Window.Current.Content to the resulting control from the Func<T>, everything works just great.

This is what makes it so perplexing.

callummoffat commented 8 years ago

I don't know how to fix it. @bc3tech, what does cause the crash? You've studied this issue more than I have.

bc3tech commented 8 years ago

I honestly have no idea. I've searched the interwebs and tried other things w/in my code. I can only say I do not think this is a Template 10-caused issue. I think there's something we have found yet, I'm just not sure what it is. The more curious part is how implementing the Splash in the way that I did in #715 works just fine with the same Control being put in Window.Current.Content

callummoffat commented 8 years ago

OK, then. There is one way to confirm whether or not this issue is Template 10-related. I'll downgrade T10 version by version, checking to see whether I get this crash. From that information, we can deduce roughly where the problem started happening, and hopefully resolve it.

bc3tech commented 8 years ago

anxious to hear your findings, I tried the same thing to no avail. :/ If you look at the source, the line that's blowing up has been in T10 for 6+ months

callummoffat commented 8 years ago

Didn't you find that this started happening in 1.1.5? I'll take a look at the changelog, and see what has been updated since then.

callummoffat commented 8 years ago

Bingo. There were several fixes in 1.1.5 that could be the cause of our troubles. The most likely one is related to "correcting double navigation".

@JerryNixon, could these issues be a side-effect of the double navigation correction fixes?

bc3tech commented 8 years ago

@callummoffat did you roll back to v1.1.4 and have no issues? could you link to the commit you're citing?

callummoffat commented 8 years ago

No. I'm getting an error saying "The parameter is incorrect". No other information.

bc3tech commented 8 years ago

right. so same error prior to 1.1.5.

callummoffat commented 8 years ago

Hey @JerryNixon,

Do you have any ideas with regards to this bug? It's a real head-scratcher.

darenm commented 8 years ago

I suspect it is a platform issue, not a T10 issue. I have had a number of instances of similar issues on Windows 10 Mobile (with and without Template 10) where changing out key containers, etc. cause fatal issues. You may need to consider a different approach where you don't change the Window.Current.Content but instead swap content within the shell.

I also believe Jerry posted the other day that he is tied up with work for a while so he wouldn't be active.

bc3tech commented 8 years ago

Good info, @darenm I am curious, though, why the SplashFactory approach is bombing yet my workaround has no issues. it really appears as though the same exact thing is happening yet it errors out in one case and not the other.

darenm commented 8 years ago

Yes - and that is what bedevils me in the current world where we cannot peer into the underlying system with any clarity. I suspect timing, threading and asynchrony play significant roles in the issues such as this and they manifest more frequently on Mobile due to the proportional lack of power of those devices. I often find that the problems do not occur in debug or with the debugger attached, but do in release mode, etc. I don't have a silver bullet here - just sharing observations that may help make the decision that expending significant effort to "solve" the problem may be better spent investigating an alternative approach.

Opiumtm commented 8 years ago

@darenm I can say some words about release mode and why release mode bring us more bugs. Release builds for UWP are compiled using .NET Native toolchain stack (instead of classic .NET). As I already experienced, .NET Native still is not mature technology when compared to classic .NET. .NET Native compiled code behave somewhat different in parallelism and async related areas. For example, infinite loop even on background thread should freeze .NET Native compiled program. It looks crazy (after all, background threads are intended to work parallel with foreground thread), but there is a vague issue. Garbage Collector with .NET Native compiled code can not block execution of threads in this situation to do its work. .NET Native allow Garbage Collector to intercept execution flow only at a compile-time known points of execution. When your code make call to any not inlined method, this point is a place when Garbage Collector can intercept execution flow and safely collect garbage without breaking pointers to objects (GC can move objects on heap and so, if pointer to object changed at inappopriate time, your code will crash) only at certain points of code. With classical .NET JIT compiler there is no problem here - code execution can be intercepted at run time. But with .NET Native there is a problem. Right now when you execute on any thread infinite loop without not inlined calls to any other methods, Garbage Collector stop any other threads at certain points of interception and will infinite wait for looped thread to intercept execution and as thread will never reach this point, entire application goes into a fatal deadlock. This problem can be solved (after all, it was solved in .NET classic ngen native image generator), but with current version of .NET Native toolchain we have this nasty issue, this infinite loop threading issue is openly admitted by .NET Native toolchain team and well documented on MSDN.

So, .NET Native Toolchain is certainly not a mature technology. It have many async related issues and does not behave correctly in many async logic cases. With my application I am already very tired of .NET Native threading and async behavior. Quality of async/await and threading in .NET Native is miserable. There are many sudden freezes, random timing instability, weird parallel behavior and so on. When same .NET parallel code is executed on classic .NET runtime - no problems. When it is compiled with .NET Native - many vague and weird issues. Most often that issues are tolerable (we can tolerate short-timed freezes and timing instability in vast majority of cases - it looks ugly, but does not break things). But when we have non trivial complex parallel code with many synchronizations and carefully placed timing - that code most probably will fail because of .NET Native. Sad, but true. All that complex parallel code must be carefully debugged in release mode because .NET Native treat parallelism differently and with much worse quality of service.

callummoffat commented 8 years ago

I'm just trying a change in my app's code:

Based on what you've told me, @darenm, I think that this fix is worth attempting.

callummoffat commented 8 years ago

Nope, my attempted fix resulted in no change. I think these kind of glitches badly need cleaning up. If you're right, and these crashes are platform-related, then Windows 10 Mobile needs a serious spring clean.

darenm commented 8 years ago

Are you still trying to change Windows.Current.Content?

callummoffat commented 8 years ago

Unfortunately, I can't work around changing Window.Current.Content. My app requires authentication using a Microsoft Account. I can't figure out a way to use a custom UI for authentication, so I'm forced to use WebAuthenticationBroker. On Mobile, this class updates Window.Current.Content temporarily to provide an authentication UI. There doesn't seem to be a method of providing a custom UI for WebAuthenticationBroker.

callummoffat commented 8 years ago

This bug is... weird. The problem was to do with me using UserControl's for some of my UI, and binding it to my view model. Putting the UI back into the page, instead of placing it into a UserControl, solved this issue for me. @darenm, I believe you're right about this being platform-related, rather than due to T10.