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

[iOS][net6] Tried to create a managed reference from an object that already has a managed reference #8688

Open
jeromelaban opened this issue May 4, 2022 · 18 comments · Fixed by #8854 or #8887
Labels
area/catalyst 🍏 Categorizes an issue or PR as relevant to mac Catalyst difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools

Comments

@jeromelaban
Copy link
Member

Current behavior

Run an net6.0-ios app (e.g. Uno.Gallery) and you notice error messages like this one:

Uno.Gallery.Mobile[66421:2250932] Tried to create a managed reference from an object that already has a managed reference (type: Windows.UI.Xaml.Controls.FontIcon)

Expected behavior

No error messages.

How to reproduce it (as minimally and precisely as possible)

No response

Workaround

None.

Works on UWP/WinUI

No response

Environment

Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia

NuGet package version(s)

4.2.6

Affected platforms

iOS

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

@jeromelaban jeromelaban added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools area/catalyst 🍏 Categorizes an issue or PR as relevant to mac Catalyst and removed triage/untriaged Indicates an issue requires triaging or verification labels May 4, 2022
@spouliot
Copy link
Contributor

spouliot commented May 4, 2022

I've seen similar messages in macOS legacy (pre-net6)

2022-05-04 11:43:29.037 SamplesApp.macOS[59225:19795331] Tried to create a managed reference from an object that already has a managed reference (type: Windows.UI.Xaml.Controls.Border)
2022-05-04 11:43:29.039 SamplesApp.macOS[59225:19795331] Tried to create a managed reference from an object that already has a managed reference (type: Windows.UI.Xaml.Controls.Border)

@spouliot
Copy link
Contributor

This is printed when the runtime finds out there's two managed instances for a single native one.

This should have a very limited performance impact (unless it happens a lot) since managed instances are generally lightweight (versus the native instance).

This situation can happen (it's actually needed) with Xamarin SDK bindings, most are stateless bindings (wrappers) and different rules (like recreating managed peers on demand) are possible for such bindings, including not printing this warning. IsDirectBinding is true for such instances.

Now "user types" can have extra state/logic in managed code so having more than one managed instance associated to a native one can lead to weird situations at runtime. IsDirectBinding is false for such instances.

The warning is kind new'ish (a year old) and was introduced with coreclr support (even if it's printed for mono too)
xamarin/xamarin-macios#11271

Before that PR it only existed in special (local) builds to debug refcounting
xamarin/xamarin-macios@5d42c93#diff-8bbfcca4e49fcf01536b6f7c128f4dc9c9bdf33914bea0841a8875c81944f63aL2016

IOW the condition(s) leading to this warning being printed likely existed inside Uno for a long time. I'll try to identify the Uno code that triggers this...

@jeromelaban
Copy link
Member Author

jeromelaban commented May 18, 2022

I'm wondering if this could be related to this portion:

https://github.com/unoplatform/uno/blob/master/src/SourceGenerators/Uno.UI.SourceGenerators/DependencyObject/DependencyObjectGenerator.cs#L601-L634

where we are handling memory management explicitly to avoid objects lingering too much in memory. This is used avoid UIKit to re-create managed wrappers when children have lost their managed parent counter part, but UIKit still invokes overriden methods.

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/DependencyObjectGenerator.cs at master · unoplatform/uno

@spouliot
Copy link
Contributor

Quite likely related! I have this generated code in my call stack before this is printed.

Windows.UI.Xaml.UIElement..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/UIElement.macOS.cs:33
Windows.UI.Xaml.FrameworkElement..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOSmacOS.cs:39
Windows.UI.Xaml.Controls.Border..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/Controls/Border/Border.iOSmacOS.cs:32
Uno.UI.Controls.BindableNSView.Dispose(bool disposing) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:196
Uno.UI.Controls.BindableNSView.<Dispose>b__62_0(Windows.UI.Core.IdleDispatchedHandlerArgs _) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:207
Windows.UI.Core.CoreDispatcher.(Uno.UI.Dispatching.IdleDispatchedHandlerArgs c) in /Users/poupou/git/external/uno/uno/src/Uno.UWP/UI/Core/CoreDispatcher.cs:88
Windows.UI.Xaml.Controls.Border..ctor(string[] args) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/Controls/Border/Border.iOSmacOS.cs:32

@jeromelaban
Copy link
Member Author

That is interesting, this would mean we're rematerializing objects that were already collected. GC is hard :)

@spouliot
Copy link
Contributor

yes v.RemoveFromSuperview(); is where it happens

The good news is that this likely means there not much chance this could break things.
The bad news is that it makes disposal more expensive (than it has to be).

I'm updating/rebuilding xamarin-macios to see the order of disposal...

@spouliot
Copy link
Contributor

🧠 dump

I disabled a few things that could have interfered but that did not help. Also made a small test case, using only XI, does not hit the same condition. More investigation will be needed.

Also replacing v.RemoveFromSuperview(); with Subviews = Array.Empty<AppKit.NSView>(); solves the problem - but that's only possible with AppKit since UIView.Subviews is readonly.

@spouliot
Copy link
Contributor

This is a Xamarin runtime issue where calling GC.ReRegisterForFinalize(this); does not reset the NSObjectFlagsInFinalizerQueue of the instance. This leads to the creation of a 2nd instance and creating the GCHandle for it detects the duplicate and warns about it.

Filed as xamarin/xamarin-macios#15089

@workgroupengineering
Copy link
Contributor

can issue #8682 be related to this issue?

@spouliot
Copy link
Contributor

can issue #8682 be related to this issue?

@workgroupengineering it could be.

The crash is different and many things can lead to such crash. However the (azure) logs shows there was a problem with the same code just before the crash. That makes it a good suspect but there's likely something else too involved (like a call to a disposed instance).

fail: Uno.UI.Dispatching.CoreDispatcher[0]
      Dispatcher unhandled exception
      ObjCRuntime.RuntimeException: Failed to marshal the Objective-C object 0x7fdd27928ae0 (type: MUXControlsTestApp_DropDownButtonPage). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance (because the type 'MUXControlsTestApp.DropDownButtonPage' does not have a constructor that takes one IntPtr argument).
        at ObjCRuntime.Runtime.MissingCtor (System.IntPtr ptr, System.IntPtr klass, System.Type type, ObjCRuntime.Runtime+MissingCtorResolution resolution) [0x00042] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1088 
        at ObjCRuntime.Runtime.ConstructNSObject[T] (System.IntPtr ptr, System.Type type, ObjCRuntime.Runtime+MissingCtorResolution missingCtorResolution) [0x0002a] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1117 
        at ObjCRuntime.Runtime.GetNSObject[T] (System.IntPtr ptr) [0x00103] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1261 
        at AppKit.NSView.get_Superview () [0x0002c] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/build/mac/mobile/AppKit/NSView.g.cs:10223 
        at AppKit.NSView.RemoveFromSuperview () [0x00007] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/build/mac/mobile/AppKit/NSView.g.cs:4084 
        at Uno.UI.Controls.BindableNSView.Dispose (System.Boolean disposing) [0x0003c] in /Users/runner/work/1/s/src/Uno.UI/obj/Release/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:219 
        at Foundation.NSObject.Dispose () [0x00001] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/Foundation/NSObject2.cs:144 
        at Uno.UI.Controls.BindableNSView.<Dispose>b__64_0 (Windows.UI.Core.IdleDispatchedHandlerArgs _) [0x00000] in /Users/runner/work/1/s/src/Uno.UI/obj/Release/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:230 
        at Windows.UI.Core.CoreDispatcher+<>c__DisplayClass21_0.<RunIdleAsync>b__0 (Uno.UI.Dispatching.IdleDispatchedHandlerArgs c) [0x00000] in /Users/runner/work/1/s/src/Uno.UWP/UI/Core/CoreDispatcher.cs:88 
        at Uno.UI.Dispatching.CoreDispatcher+<>c__DisplayClass38_0.<RunIdleAsync>b__0 () [0x00000] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:199 
        at Uno.UI.Dispatching.CoreDispatcher.InvokeOperation (Uno.UI.Dispatching.UIAsyncOperation operation) [0x00058] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:373 
        at Uno.UI.Dispatching.CoreDispatcher.InvokeOperationSafe (Uno.UI.Dispatching.UIAsyncOperation operation) [0x00000] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:337 
fail: Uno.UI.Dispatching.CoreDispatcher[0]
      Dispatcher unhandled exception
      ObjCRuntime.RuntimeException: Failed to marshal the Objective-C object 0x7fdd22fc9760 (type: Uno_UI_Samples_UITests_ImageTestsControl_LoadFromBytes). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance (because the type 'Uno.UI.Samples.UITests.ImageTestsControl.LoadFromBytes' does not have a constructor that takes one IntPtr argument).
        at ObjCRuntime.Runtime.MissingCtor (System.IntPtr ptr, System.IntPtr klass, System.Type type, ObjCRuntime.Runtime+MissingCtorResolution resolution) [0x00042] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1088 
        at ObjCRuntime.Runtime.ConstructNSObject[T] (System.IntPtr ptr, System.Type type, ObjCRuntime.Runtime+MissingCtorResolution missingCtorResolution) [0x0002a] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1117 
        at ObjCRuntime.Runtime.GetNSObject[T] (System.IntPtr ptr) [0x00103] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:1261 
        at AppKit.NSView.get_Superview () [0x0002c] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/build/mac/mobile/AppKit/NSView.g.cs:10223 
        at AppKit.NSView.RemoveFromSuperview () [0x00007] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/build/mac/mobile/AppKit/NSView.g.cs:4084 
        at Uno.UI.Controls.BindableNSView.Dispose (System.Boolean disposing) [0x0003c] in /Users/runner/work/1/s/src/Uno.UI/obj/Release/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:219 
        at Foundation.NSObject.Dispose () [0x00001] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/Foundation/NSObject2.cs:144 
        at Uno.UI.Controls.BindableNSView.<Dispose>b__64_0 (Windows.UI.Core.IdleDispatchedHandlerArgs _) [0x00000] in /Users/runner/work/1/s/src/Uno.UI/obj/Release/xamarinmac20/g/DependencyObjectGenerator/Uno_UI_Controls_BindableNSView_ce660da041c6d0cda3edb86409904a1d.g.cs:230 
        at Windows.UI.Core.CoreDispatcher+<>c__DisplayClass21_0.<RunIdleAsync>b__0 (Uno.UI.Dispatching.IdleDispatchedHandlerArgs c) [0x00000] in /Users/runner/work/1/s/src/Uno.UWP/UI/Core/CoreDispatcher.cs:88 
        at Uno.UI.Dispatching.CoreDispatcher+<>c__DisplayClass38_0.<RunIdleAsync>b__0 () [0x00000] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:199 
        at Uno.UI.Dispatching.CoreDispatcher.InvokeOperation (Uno.UI.Dispatching.UIAsyncOperation operation) [0x00058] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:373 
        at Uno.UI.Dispatching.CoreDispatcher.InvokeOperationSafe (Uno.UI.Dispatching.UIAsyncOperation operation) [0x00000] in /Users/runner/work/1/s/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:337 

@jeromelaban
Copy link
Member Author

This one is likely to be related to a collected instance indeed. It's not yet clear why though.

@spouliot
Copy link
Contributor

As suspected the condition to trigger the bug differs a bit on iOS. In that case the Superview property is also called in the override of SetNeedsLayout();.

src/Uno.UI/UI/Xaml/UIElement.cs:817

#elif XAMARIN_IOS
			SetNeedsLayout();
			SetLayoutFlags(LayoutFlag.MeasureDirty);
#elif __MACOS__
			base.NeedsLayout = true;
			SetLayoutFlags(LayoutFlag.MeasureDirty);
#endif

The call to the disposed-then-reregistered instance is showing the warning. Sadly a variation of the macOS workaround cannot be applied here.

Call Stack

The call stack is much deeper. It seems measurements are being made between the first/original Dispose(), inited by the finalizer, and the the second/dispatched Dispose() call.

Foundation.NSObject.CreateManagedRef(bool retain) in /Users/poupou/git/xamarin-macios/src/Foundation/NSObject2.cs:382
  • this is printing the warning
Foundation.NSObject.InitializeObject(bool alloced) in /Users/poupou/git/xamarin-macios/src/Foundation/NSObject2.cs:363
Foundation.NSObject..ctor(System.IntPtr handle, bool alloced) in /Users/poupou/git/xamarin-macios/src/Foundation/NSObject2.cs:218
Foundation.NSObject..ctor(System.IntPtr handle) in /Users/poupou/git/xamarin-macios/src/Foundation/NSObject2.cs:207
UIKit.UIResponder..ctor(System.IntPtr handle) in /Users/poupou/git/xamarin-macios/src/build/ios/native/UIKit/UIResponder.g.cs:77
UIKit.UIView..ctor(System.IntPtr handle) in /Users/poupou/git/xamarin-macios/src/build/ios/native/UIKit/UIView.g.cs:91
Uno.UI.Controls.BindableUIView..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/Controls/BindableUIView.iOS.cs:88
Windows.UI.Xaml.UIElement..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinios10/g/NativeCtorsGenerator/Windows_UI_Xaml_UIElement_85915f7d6b7124464cf9d270ce415b49.g.cs:38
Windows.UI.Xaml.FrameworkElement..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOSmacOS.cs:39
Windows.UI.Xaml.Controls.Panel..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinios10/g/NativeCtorsGenerator/Windows_UI_Xaml_Controls_Panel_28f728efa4d8453cbdcff486fc3ca3a7.g.cs:38
Windows.UI.Xaml.Controls.Grid..ctor(System.IntPtr handle) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinios10/g/NativeCtorsGenerator/Windows_UI_Xaml_Controls_Grid_d6e0da67eb69ca000ade21f8a12e9768.g.cs:38
System.Reflection.RuntimeConstructorInfo.InternalInvoke() in 
System.Reflection.RuntimeConstructorInfo.InternalInvoke(object obj, object[] parameters, bool wrapExceptions) in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:936
System.Reflection.RuntimeConstructorInfo.DoInvoke(object obj, System.Reflection.BindingFlags invokeAttr, System.DefaultBinder binder, object[] parameters, System.Globalization.CultureInfo culture) in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:926
ObjCRuntime.Runtime.ConstructNSObject<UIKit.UIView>(System.IntPtr ptr, System.RuntimeType type, ObjCRuntime.Runtime.MissingCtorResolution missingCtorResolution) in /Users/poupou/git/xamarin-macios/src/ObjCRuntime/Runtime.cs:1230
ObjCRuntime.Runtime.GetNSObject<UIKit.UIView>(System.IntPtr ptr) in /Users/poupou/git/xamarin-macios/src/ObjCRuntime/Runtime.cs:1446
UIKit.UIView.get_Superview() in /Users/poupou/git/xamarin-macios/src/build/ios/native/UIKit/UIView.g.cs:4425
Windows.UI.Xaml.FrameworkElement.SetSuperviewNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/Mixins/iOS/FrameworkElementMixins.g.cs:198
Windows.UI.Xaml.FrameworkElement.SetNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOS.cs:39
Windows.UI.Xaml.FrameworkElement.SetSuperviewNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/Mixins/iOS/FrameworkElementMixins.g.cs:202
Windows.UI.Xaml.FrameworkElement.SetNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOS.cs:39
Windows.UI.Xaml.FrameworkElement.SetSuperviewNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/Mixins/iOS/FrameworkElementMixins.g.cs:202
Windows.UI.Xaml.FrameworkElement.SetNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOS.cs:39
Windows.UI.Xaml.FrameworkElement.SetSuperviewNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/Mixins/iOS/FrameworkElementMixins.g.cs:202
Windows.UI.Xaml.FrameworkElement.SetNeedsLayout() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/FrameworkElement.iOS.cs:39
Windows.UI.Xaml.UIElement.InvalidateMeasure() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/UIElement.cs:817
  • This is the code, calling Superview, that I mentioned above
  • This is iOS specific (and why my macOS workaround works)
Windows.UI.Xaml.Controls.TextBlock.InvalidateTextBlock() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs:929
Windows.UI.Xaml.Controls.TextBlock.OnFontFamilyChanged() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs:264
Windows.UI.Xaml.Controls.TextBlock.<>c.<.cctor>b__285_4(Windows.UI.Xaml.Controls.TextBlock s, Windows.UI.Xaml.DependencyPropertyChangedEventArgs e) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs:257
Windows.UI.Xaml.PropertyMetadata.RaisePropertyChanged(Windows.UI.Xaml.Controls.TextBlock source, Windows.UI.Xaml.DependencyPropertyChangedEventArgs e) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/PropertyMetadata.cs:187
Windows.UI.Xaml.DependencyObjectStore.InvokeCallbacks(Windows.UI.Xaml.Controls.TextBlock actualInstanceAlias, Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, Windows.UI.Xaml.Media.FontFamily previousValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences previousPrecedence, Windows.UI.Xaml.Media.FontFamily newValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences newPrecedence, bool bypassesPropagation) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1824
Windows.UI.Xaml.DependencyObjectStore.RaiseCallbacks(Windows.UI.Xaml.Controls.TextBlock actualInstanceAlias, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, Windows.UI.Xaml.Media.FontFamily previousValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences previousPrecedence, Windows.UI.Xaml.Media.FontFamily newValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences newPrecedence) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1734
Windows.UI.Xaml.DependencyObjectStore.InnerSetValue(Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.Media.FontFamily value, Windows.UI.Xaml.DependencyPropertyValuePrecedences precedence, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, bool isPersistentResourceBinding) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:501
Windows.UI.Xaml.DependencyObjectStore.SetValue(Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.Media.FontFamily value, Windows.UI.Xaml.DependencyPropertyValuePrecedences precedence, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, bool isPersistentResourceBinding) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:435
Windows.UI.Xaml.DependencyObjectStore.OnParentPropertyChangedCallback(Uno.UI.DataBinding.ManagedWeakReference sourceInstance, Windows.UI.Xaml.DependencyProperty parentProperty, Windows.UI.Xaml.DependencyPropertyChangedEventArgs args) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1007
Windows.UI.Xaml.DependencyObjectStore.OnParentPropertyChangedCallback(Uno.UI.DataBinding.ManagedWeakReference sourceInstance, Windows.UI.Xaml.DependencyProperty parentProperty, Windows.UI.Xaml.DependencyPropertyChangedEventArgs args) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1019
Windows.UI.Xaml.DependencyObjectStore.OnParentPropertyChangedCallback(Uno.UI.DataBinding.ManagedWeakReference sourceInstance, Windows.UI.Xaml.DependencyProperty parentProperty, Windows.UI.Xaml.DependencyPropertyChangedEventArgs args) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1019
Windows.UI.Xaml.DependencyObjectStore.CallChildCallback(Windows.UI.Xaml.DependencyObjectStore childStore, Uno.UI.DataBinding.ManagedWeakReference instanceRef, Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.DependencyPropertyChangedEventArgs eventArgs) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1859
Windows.UI.Xaml.DependencyObjectStore.InvokeCallbacks(Windows.UI.Xaml.Controls.ContentPresenter actualInstanceAlias, Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, Windows.UI.Xaml.Media.FontFamily previousValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences previousPrecedence, Windows.UI.Xaml.Media.FontFamily newValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences newPrecedence, bool bypassesPropagation) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1801
Windows.UI.Xaml.DependencyObjectStore.RaiseCallbacks(Windows.UI.Xaml.Controls.ContentPresenter actualInstanceAlias, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, Windows.UI.Xaml.Media.FontFamily previousValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences previousPrecedence, Windows.UI.Xaml.Media.FontFamily newValue, Windows.UI.Xaml.DependencyPropertyValuePrecedences newPrecedence) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1734
Windows.UI.Xaml.DependencyObjectStore.InnerSetValue(Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.UnsetValue value, Windows.UI.Xaml.DependencyPropertyValuePrecedences precedence, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, bool isPersistentResourceBinding) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:501
Windows.UI.Xaml.DependencyObjectStore.SetValue(Windows.UI.Xaml.DependencyProperty property, Windows.UI.Xaml.UnsetValue value, Windows.UI.Xaml.DependencyPropertyValuePrecedences precedence, Windows.UI.Xaml.DependencyPropertyDetails propertyDetails, bool isPersistentResourceBinding) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:435
Windows.UI.Xaml.DependencyObjectStore.CleanupInheritedProperties() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1130
Windows.UI.Xaml.DependencyObjectStore.InheritedPropertiesDisposable.Dispose() in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:1083
Uno.Disposables.SerialDisposable.set_Disposable(System.IDisposable value) in /Users/poupou/git/external/uno/uno/src/Uno.Foundation/Uno.Core.Extensions/Uno.Core.Extensions.Disposables/Disposables/SerialDisposable.cs:78
Windows.UI.Xaml.DependencyObjectStore.set_Parent(object value) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs:150
Windows.UI.Xaml.FrameworkElement.WillMoveToSuperview(UIKit.UIView newsuper) in /Users/poupou/git/external/uno/uno/src/Uno.UI/Mixins/iOS/FrameworkElementMixins.g.cs:134
ObjCRuntime.Messaging.void_objc_msgSendSuper() in 
UIKit.UIView.RemoveFromSuperview() in /Users/poupou/git/xamarin-macios/src/build/ios/native/UIKit/UIView.g.cs:1261
  • This is where the macOS resurface the superview, due to how it's bindings are done (likely they were not modernized after newrefcount became the permanently enabled);
  • The iOS bindings are different (updated) so the superview is not resurfaced here.
Uno.UI.Controls.BindableUIView.Dispose(bool disposing) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinios10/g/DependencyObjectGenerator/Uno_UI_Controls_BindableUIView_283f42f32036f7bd27e98620585dab23.g.cs:236
Foundation.NSObject.Dispose() in /Users/poupou/git/xamarin-macios/src/Foundation/NSObject2.cs:226
Uno.UI.Controls.BindableUIView.<Dispose>b__59_0(Windows.UI.Core.IdleDispatchedHandlerArgs _) in /Users/poupou/git/external/uno/uno/src/Uno.UI/obj/Debug/xamarinios10/g/DependencyObjectGenerator/Uno_UI_Controls_BindableUIView_283f42f32036f7bd27e98620585dab23.g.cs:249
  • This is where the dispatched call to Dispose() is done
Windows.UI.Core.CoreDispatcher.(Uno.UI.Dispatching.IdleDispatchedHandlerArgs c) in /Users/poupou/git/external/uno/uno/src/Uno.UWP/UI/Core/CoreDispatcher.cs:88
Uno.UI.Dispatching.CoreDispatcher.() in /Users/poupou/git/external/uno/uno/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:199
Uno.UI.Dispatching.CoreDispatcher.InvokeOperation(Uno.UI.Dispatching.UIAsyncOperation operation) in /Users/poupou/git/external/uno/uno/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:373
Uno.UI.Dispatching.CoreDispatcher.InvokeOperationSafe(Uno.UI.Dispatching.UIAsyncOperation operation) in /Users/poupou/git/external/uno/uno/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:337
Uno.UI.Dispatching.CoreDispatcher.DispatchItems() in /Users/poupou/git/external/uno/uno/src/Uno.UI.Dispatching/Core/CoreDispatcher.cs:299
CoreFoundation.DispatchQueue.static_dispatcher_to_managed(System.IntPtr context) in /Users/poupou/git/xamarin-macios/src/CoreFoundation/Dispatch.cs:387
UIKit.UIApplication.UIApplicationMain() in 
UIKit.UIApplication.Main(string[] args, System.Type principalClass, System.RuntimeType delegateClass) in /Users/poupou/git/xamarin-macios/src/UIKit/UIApplication.cs:85
SamplesApp.iOS.Application.Main(string[] args) in /Users/poupou/git/external/uno/uno/src/SamplesApp/SamplesApp.iOS/Main.cs:12

There's probably some performance gains in avoiding those computations when an object is going thru disposal.

In addition such optimization would avoid the calls to Superview which would also solve the warning (and the perf cost to create new, short-lived managed peers).

@jeromelaban
Copy link
Member Author

Very interesting find! Thanks!

So we could either determine that the object in disposed state somehow to avoid this entire tree, or find a way to determine if the superview is already collected. Is there a way to do that without re-creating the managed counterpart ?

@spouliot
Copy link
Contributor

So we could either determine that the object in disposed state somehow to avoid this entire tree

There's a bool _isDisposed that could re-typed to a 3 states (e.g. an enum with Alive, Disposing and Disposed members).

That could be used to skip the measuring code when != Alive is set.

Since that would be located (and set) inside Dispose and checked in a few* other places this should continue to work even after Microsoft fix the issue.

* I'm just not sure how many places would need such a check... but it's easy to try the one I have in the stack trace 😄

or find a way to determine if the superview is already collected.

The flags are not exposed publicly by Xamarin SDKs. It would require reflection (quite slow) or some unsafe code that assume the current memory layout of NSObject (unsafe as named).

Ideally Microsoft will fix this 🤞, e.g. get notified if the instance is re-registered, and this won't be an issue in the future.

There's a third option: That Uno Dispose code is at least four years old (git blame can't see before the initial code import) and if it predates newrefcount then it might not be required anymore.

For net6-macos, which uses CoreCLR, it might also be counter productive to run this code.

It might be a good idea to measure again it's impact and, if possible, simplify or even remove it.

I'll start with the first option to see how self-contained the fix would be...

@spouliot
Copy link
Contributor

So we cannot just avoid the condition - it has other merits but it does not workaround the resurfacing issue (since the order of disposal is not under our control).

What seems to be working is using Unsafe to reset the flag after the call to GC.ReRegisterForFinalize(this); like

const byte InFinalizerQueue = 16; // see NSObject2.cs
var poker = Unsafe.As<NSObjectMemoryRepresentation>(this);
poker.flags = (byte)(poker.flags & ~InFinalizerQueue);

Since the state was corrected the call to GetNSObject returns the original managed instance (instead of creating a new one) so the warning is not shown.

I'll clean my code and split this into 2 or 3 consecutive PRs. That will make it easier to revert the hack'ish (above) part while keeping the other improvements.

spouliot added a commit to spouliot/uno that referenced this issue May 23, 2022
…ss common

- avoid generating disable `Dispose` code for Android
- inline `RequestCollect` method (if only to reduce metadata)
- reuse the `Count` check to avoid the enumerator (iOS)
- use the macOS r/w `Subviews` property to avoid multiple native calls
workgroupengineering pushed a commit to workgroupengineering/uno that referenced this issue May 23, 2022
…ss common

- avoid generating disable `Dispose` code for Android
- inline `RequestCollect` method (if only to reduce metadata)
- reuse the `Count` check to avoid the enumerator (iOS)
- use the macOS r/w `Subviews` property to avoid multiple native calls
workgroupengineering pushed a commit to workgroupengineering/uno that referenced this issue May 23, 2022
…ss common

- avoid generating disable `Dispose` code for Android
- inline `RequestCollect` method (if only to reduce metadata)
- reuse the `Count` check to avoid the enumerator (iOS)
- use the macOS r/w `Subviews` property to avoid multiple native calls
workgroupengineering pushed a commit to workgroupengineering/uno that referenced this issue May 24, 2022
…ss common

- avoid generating disable `Dispose` code for Android
- inline `RequestCollect` method (if only to reduce metadata)
- reuse the `Count` check to avoid the enumerator (iOS)
- use the macOS r/w `Subviews` property to avoid multiple native calls
jeromelaban added a commit that referenced this issue May 24, 2022
refactor: Tweak generated `Dispose(bool)` to make #8688 less common
@MartinZikmund
Copy link
Member

Noticed this warning is showing up all the time when running iOS runtime tests - probably should be checked to see if it is a regression or not

@nickrandolph
Copy link
Contributor

@cconner is seeing this, so would be good to see if this is a regression

@jeromelaban
Copy link
Member Author

This is not a regression, this message is not having any incidence on the app behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/catalyst 🍏 Categorizes an issue or PR as relevant to mac Catalyst difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools
Projects
None yet
5 participants