xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.48k stars 514 forks source link

warning: Tried to create a managed reference from an object that already has a managed reference #15089

Open spouliot opened 2 years ago

spouliot commented 2 years ago

Steps to Reproduce

  1. an NSObject subclass is finalized
  2. this calls object_queued_for_finalization and set NSObjectFlagsInFinalizerQueue in the instance's flags
  3. the call to Dispose(false) calls GC.ReRegisterForFinalize(this);
  4. that has no effect (notification/callback) to change the instance's flags so it's still has NSObjectFlagsInFinalizerQueue set
  5. something calls the instance itself (e.g. RemoveFromSuperview)
  6. GetNSObject is used to get the instance, TryGetNSObject looks for one using evenInFinalizerQueue: false
  7. since NSObjectFlagsInFinalizerQueue is set the method returns null
  8. however the call xamarin_set_gchandle_with_flags_safe will find a GCHandle (because a managed peer exists) and return false
  9. that will make the warning appears in the logs

Expected Behavior

The warning should not be printed and it the condition it checks should not occur.

Actual Behavior

The logs shows the warning Tried to create a managed reference from an object that already has a managed reference. Some NSObject instances are resurfaced needlessly since they already exists in managed land.

Environment

The issue exists in master

Notes

Build Logs

Not useful.

Example Project (If Possible)

It's easier to repro using Xamarin.Mac since NSView.RemoveFromSuperview calls the Superview property. However the warning is also seen on iOS but the conditions might be a little different.

Building Uno SampleApp for macOS and selecting the Border sample will print the warning. The generated Dispose can be seen in the generator code.

If needed I'll try to create a smaller sample. I had no luck earlier (on iOS) but I had not yet found the root issue.

spouliot commented 2 years ago

The fact it's easier to dupe on macOS is because the bindings include custom code

[Export ("removeFromSuperview")]
[PreSnippet ("var mySuper = Superview;", Optimizable = true)]
[PostSnippet ("if (mySuper != null) {\n\t#pragma warning disable 168\n\tvar flush = mySuper.Subviews;\n#pragma warning restore 168\n\t}", Optimizable = true)]
void RemoveFromSuperview ();

AFAIK this is not needed anymore (since newrefcount become the default with the unified API). However I can't see/blame the source code from the era to confirm. If that's the case then NSView has a few more places that needs scrubbing.

spouliot commented 2 years ago

Quick followup

On iOS the warning appears because there are calls to the Superview property (in between the first Dispose() (from finalizer) and the second Dispose() calls (dispatched from the first one).

IOW the call to RemoveFromSuperview is not the trigger of the problem on iOS, just on macOS. This confirms the extra, custom code on the macOS side is triggering the issue.

reference: https://github.com/unoplatform/uno/issues/8688#issuecomment-1133800352

spouliot commented 2 years ago

Using the following code, just after the call to GC.ReRegisterForFinalize(this); solves the issue. IOW the original managed instance is returned by GetNSObject.

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

However I can't see/blame the source code from the era to confirm.

It was added here: https://gist.github.com/rolfbjarne/b44c6e86bca3a9f46997a4d8f6a1aaf9

rolfbjarne commented 2 years ago

I created a PR to remove those Pre/Post snippets: #15103.

That doesn't fix the underlying problem though (resurrected managed peers only half resurrected), so I'll leave this bug open for now.

spouliot commented 2 years ago

note: this does not seems to be an issue for net6-macos, which uses coreclr and a different mechanism to get notified