Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After upgrading to ReactiveUI 6.0, WPF bindings break after about 10 seconds after application startup #661

Closed
patroza opened this issue Jul 11, 2014 · 16 comments
Labels

Comments

@patroza
Copy link
Contributor

@patroza patroza commented Jul 11, 2014

First off, congrats on the major new release!!

I realize the error description is rather vague and does not really help, but it's a large app and I have no idea yet about the source of the problem, still trying to find whats going on :S

After about 10 seconds bindings seem to randomly break and my application no longer functions properly. Any suggestions what this might be?
(Still using legacy reactivecommand btw).

Thanks

@shiftkey

This comment has been minimized.

Copy link
Contributor

@shiftkey shiftkey commented Jul 11, 2014

Any exceptions in logs (or the event viewer) that might shed some light on the situation?

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 11, 2014

Sadly non so far, i've also been tracking the trace output in debug mode, and could not find a relevant exception (sadly I have a bunch of bindings that are not valid for certain types of data models, which makes this more tedious than needed).

Looking into trying to make an isolated repro, but im guessing it's a more nested detail in my application.

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

I did get the following exception now which crashed the app, I am unsure if it is related:

2014-07-12 02:03:58.2930|ERROR|Six.Core.Logging.MainLog|Abnormal termination: Type: System.InvalidCastException
Message:
    Unable to cast object of type 'ReactiveUI.ReactivePropertyChangedEventArgs`1[ReactiveUI.IReactiveObject]' to type 'ReactiveUI.IReactivePropertyChangedEventArgs`1[ReactiveUI.ReactiveObject]'.
Source: System.Reactive.Linq
TargetSite: Void OnNext(TSource)
Data:     [System.Object=]
StackTrace:
       at System.Reactive.Linq.ObservableImpl.Cast`2._.OnNext(TSource value)
    --- End of stack trace from previous location where exception was thrown ---
       at System.Reactive.PlatformServices.ExceptionServicesImpl.Rethrow(Exception exception)
       at System.Reactive.Stubs.<.cctor>b__1(Exception ex)
       at System.Reactive.AnonymousSafeObserver`1.OnError(Exception error)
       at System.Reactive.ScheduledObserver`1.Run(Object state, Action`1 recurse)
       at System.Reactive.Concurrency.Scheduler.<>c__DisplayClass50`1.<InvokeRec1>b__4d(TState state1)
       at System.Reactive.Concurrency.Scheduler.InvokeRec1[TState](IScheduler scheduler, Pair`2 pair)
       at System.Reactive.Concurrency.DispatcherScheduler.<>c__DisplayClass1`1.<Schedule>b__0()
       at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
       at MS.Internal.Threading.ExceptionFilterHelper.TryCatchWhen(Object source, Delegate method, Object args, Int32 numArgs, Delegate catchHandler)
       at System.Windows.Threading.DispatcherOperation.InvokeImpl()
       at System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(Object state)
       at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
       at System.Windows.Threading.DispatcherOperation.Invoke()
       at System.Windows.Threading.Dispatcher.ProcessQueue()
       at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
       at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
       at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
       at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
       at MS.Internal.Threading.ExceptionFilterHelper.TryCatchWhen(Object source, Delegate method, Object args, Int32 numArgs, Delegate catchHandler)
       at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
       at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
       at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
       at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
       at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
       at System.Windows.Threading.Dispatcher.Run()
       at System.Windows.Application.RunDispatcher(Object ignore)
       at System.Windows.Application.RunInternal(Window window)
       at System.Windows.Application.Run(Window window)
       at System.Windows.Application.Run()
@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

Not getting much anywhere with trying to debug this issue so far 👎
I'm guessing it is related to dropping automatic marshalling to ui thread, but I've already tried adding .ObserveOn(RxApp.MainThreadScheduler) on commands and ToProperty to no avail.

Caliburn.Micro starts to act up and say it can't find methods bound by ActionMessage, even though they work fine in RXUI 5.5.

I've tried attaching to DataContext changed events, but that also hasn't resulted in anything useful either, datacontects seem to be fine.
Properties seem to be updated properly too, it is just that WPF does not seem to respond to the changes anymore at some point..

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

I've found something slightly interesting;

If I add a DataContextChanged event to my ShellView, and in that handler attach PropertyChanged to my ViewModel, this PropertyChanged handler is never fired.

But if I store my ShellViewModel into a static global variable, and access that from my ShellView's constructor to attach the PropertyChanged event, then this event fires correctly, and I can even see it fires when I click different buttons in the UI changing state in the ViewModel.
But the UI still does not reflect these changes.

If I then update a dependency property on the View with this data, that is actually properly reflected in the UI.

I am guessing there are errors occurring while raising some of the handlers, but they seem to be silently dropped..

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

After dropping ReactiveObject as a base class, and using my own propertychanged base class, and replicating ToProperty helper, and Changed observable, all is seemingly working fine again.

So the problem seems to be in ReactiveUI's PropertyChanged implementation.
(Listening to ReactiveObjects ThrownExceptions yields nothing btw)

As I am not able to break it so far in basic repro projects, I wonder if it is threading related - property changes occuring from different/multiple threads breaking something..
Another suspect i have is the weakevent manager which checks if objects are still alive before dispatching events.

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

When the bindings break, the WeakReference to OriginalHandler, returns IsAlive=false, and then the associated WeakHandler is removed from the WeakHandler list.

The TEventHandler in this case is:

Target: {System.ComponentModel.PropertyChangedEventManager}
Method: {Method = {Void OnPropertyChanged(System.Object, System.ComponentModel.PropertyChangedEventArgs)}}

That eventmanager is also a WeakEventManager as seen here; http://referencesource.microsoft.com/#WindowsBase/src/Base/System/ComponentModel/PropertyChangedEventManager.cs

Would that be the problem? Weak on Weak = dissolves itself at some point? :)

@tberman

This comment has been minimized.

Copy link
Member

@tberman tberman commented Jul 12, 2014

So, in <RxUI6, if you had a subscription to a ReactiveObject's Changed IO, you kept that ReactiveObject alive, just by virtue of that subscription.

In RxUI6, that is no longer the case, because all of that subscription state is kept inside a ConditionalWeakMap.

My guess is that you were relying on the subscription to keep the object alive and that is no longer happening, which is why ~10 seconds in random GC occurs, and everything gets cleaned up.

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

Thanks tberman, but I don't believe this is the case here.

The Target of the handler is WPF's PropertyChangedEventManager, so it is the WPF framework that attaches itself to the event, that must be the reason why it just seems to be the View that is no longer in sync.

When applying the following hack to the WeakHandler constructor everything works as normal:

                if (handler.Target.ToString() == "System.ComponentModel.PropertyChangedEventManager")
                    this.hardReference = originalHandler;

(The reason im using ToString here is that the ReactiveUI project does not have the necessary assembly references to access that class, or it's parent: WeakEventManager: http://referencesource.microsoft.com/#WindowsBase/src/Base/System/Windows/WeakEventManager.cs

This leads me to believe that having a WeakReference operate on another WeakReference, leads to them cancelling eachother out :)

Which makes me wonder if it really should be the 'server' (the object raising the events) leveraging a WeakReference model, as opposed to the 'client' (the objects subscribing to the events).
As you can't really know if the clients are using a Weak Reference model.

Information found at http://www.codeproject.com/Articles/29922/Weak-Events-in-C#heading0010
seems to confirm my findings - where source-based weak event patterns require the listeners to keep a strong reference, which is not the case with listeners implemented with weakreferences :)

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

A slightly more elegant workaround could be:

                if (TargetIsWeak(handler))
                    hardReference = originalHandler;
            static bool TargetIsWeak(Delegate handler) {
                var type = handler.Target.GetType();
                while ((type = type.GetTypeInfo().BaseType) != null) {
                    if (type.FullName == "System.Windows.WeakEventManager")
                        return true;
                }
                return false;
            }

But still horrible of course..

@patroza patroza mentioned this issue Jul 12, 2014
1 of 2 tasks complete
@ghost

This comment has been minimized.

Copy link

@ghost ghost commented Jul 12, 2014

Thanks for digging in to this @Sickboy. I am getting a very similar experience after upgrade. WPF property bindings fail to update controls after some time, and since the subscription is not in my code it is out of my hands.

@patroza

This comment has been minimized.

Copy link
Contributor Author

@patroza patroza commented Jul 12, 2014

I've made it a pull request although it is a mediocre workaround at best I think :)

@automatonic no problem! Perhaps the workaround can get you going - Sadly replacing a NuGet package dll is cumbersome especially if you have a lot of projects depending on it, and use package restore. I went and powergrepped the csproj references and replaced them with a local compiled fixed version for the time being ;-)

@anaisbetts

This comment has been minimized.

Copy link
Member

@anaisbetts anaisbetts commented Jul 15, 2014

@jlaanstra Can you have a look at this?

@jlaanstra

This comment has been minimized.

Copy link
Member

@jlaanstra jlaanstra commented Jul 15, 2014

Looking into it.

@anaisbetts

This comment has been minimized.

Copy link
Member

@anaisbetts anaisbetts commented Jul 16, 2014

Fixed by #672

@anaisbetts anaisbetts closed this Jul 16, 2014
rhautefeuille added a commit to rhautefeuille/ReactiveUI that referenced this issue Jun 29, 2017
Fixed lost INPC subscriptions
Basically this fix assumed Net45 == WPF but it isn't true for this fork
reactiveui#661

Change-Id: If8d953c3882875c0703e8ba3c9dbae309060bfa3
@ElijahReva

This comment has been minimized.

Copy link
Contributor

@ElijahReva ElijahReva commented May 8, 2018

Guys, I see that same issue again when using ReactiveUI 8.0.0 on .net 4.7.1.

PropertyChangedEventManager.DeliverEvent got called at WPF.

@lock lock bot added the outdated label Jun 25, 2019
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.