NSArrayController.Remove destroys unrelated empty NSArray

snechaev commented 4 years ago

Steps to Reproduce

Run a project attached
Click a button in the main window
Observe that NSArray.Handle is non-zero before calling NSArrayController.Remove and became zero just after the call to Remove.

Minimal test code

            NSArray Array = new NSArray();

            var alert = new NSAlert();
            alert.InformativeText = "ViewController.Array.Handle before arrayControllerUnbinded.Remove: " + Array.Handle;
            alert.RunModal(); //Handle is non-zero

            var controller = new NSArrayController();
            controller.Remove(controller.ArrangedObjects());

            alert.InformativeText = "ViewController.Array.Handle after arrayControllerUnbinded.Remove: " + Array.Handle;
            alert.RunModal(); //Handle is zero

Expected Behavior

Operations with NSArrayController do not affect any objects, which is not related to this array controller.

Actual Behavior

Calling NSArrayController.Remove leads to unrelated NSArray being destroyed (NSArray.Handle=0).

Additional details

There is NSArray local variable in the button handler and NSArrayController. The NSArray variable is initialised with an empty NSArray. The Array controller do not have any bindings and other connections, so it is completely unrelated to the NSArray variable mentioned above. The button handler calls Remove on the ArrayController in form of arrayControllerUnbinded.Remove(arrayControllerUnbinded.ArrangedObjects()); After this call the NSArray variable Handle become zero.

Some additional notes

the NSArray may be a local variable, field or property (with or without Export attribute) - the problem is observed in all cases.
the NSArray and NSArrayController may be in the different classes/xibs - the problem will still persist
the problem does not observed if the NSArray is not empty
the problem does not observed for NSMutableArray (whatever empty or not)

Environment

=== Visual Studio Community 2019 for Mac ===

Version 8.6.2 (build 6)
Installation UUID: 106edc06-c1ac-4a89-9783-ea18c79458d1
    GTK+ 2.24.23 (Raleigh theme)
    Xamarin.Mac 6.18.0.23 (d16-6 / 088c73638)

    Package version: 612000090

=== Mono Framework MDK ===

Runtime:
    Mono 6.12.0.90 (2020-02/d3daacdaa80) (64-bit)
    Package version: 612000090

=== Roslyn (Language Service) ===

3.6.0-3.20210.9+4eafdcb1bcbd8d3573f2ba6065e56d9b9ce4f8a3

=== NuGet ===

Version: 5.6.0.6591

=== .NET Core SDK ===

SDK: /usr/local/share/dotnet/sdk/3.1.401/Sdks
SDK Versions:
    3.1.401
    3.1.302
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/msbuild/Current/bin/Sdks

=== .NET Core Runtime ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
    3.1.7
    3.1.6
    2.1.21
    2.1.20

=== Xamarin.Profiler ===

Version: 1.6.15.68
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Updater ===

Version: 11

=== Apple Developer Tools ===

Xcode 11.7 (16142)
Build 11E801a

=== Xamarin.Mac ===

Version: 6.20.2.2 (Visual Studio Community)
Hash: 817b6f72a
Branch: d16-7
Build date: 2020-07-18 18:44:59-0400

=== Xamarin.iOS ===

Xamarin.iOS not installed.
Can't find mtouch or the Version file at /Library/Frameworks/Xamarin.iOS.framework/Versions/Current.

=== Xamarin Designer ===

Version: 16.6.0.329
Hash: d4f8bcd13
Branch: remotes/origin/d16-6
Build date: 2020-04-24 02:16:02 UTC

=== Xamarin.Android ===

Not Installed

=== Microsoft Mobile OpenJDK ===

Java SDK: Not Found

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Android SDK Manager ===

Version: 16.6.0.50
Hash: 5901879
Branch: remotes/origin/d16-6
Build date: 2020-04-30 04:01:22 UTC

=== Android Device Manager ===

Version: 16.6.0.95
Hash: 45d17b5
Branch: remotes/origin/d16-6
Build date: 2020-04-30 04:01:42 UTC

=== Build Information ===

Release ID: 806020006
Git revision: c742dc3fcbf76855738eef57ce41d65f91acbf27
Build date: 2020-05-28 15:22:51-04
Build branch: release-8.6
Xamarin extensions: c742dc3fcbf76855738eef57ce41d65f91acbf27

=== Operating System ===

Mac OS X 10.15.6
Darwin 19.6.0 Darwin Kernel Version 19.6.0
    Thu Jun 18 20:49:00 PDT 2020
    root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64

=== Enabled user installed extensions ===

FileNesting 0.1.2

Build Logs

Build log

Building 44 (Debug) Build started 07.09.2020 17:22:03. __________________________________________________ Project "/Users/sergey/work/44/44/44.csproj" (Build target(s)): Target _BeforeCoreCompileImageAssets: Skipping target "_BeforeCoreCompileImageAssets" because all output files are up-to-date with respect to the input files. Target _CoreCompileImageAssets: Skipping target "_CoreCompileImageAssets" because all output files are up-to-date with respect to the input files. Target _CoreCompileInterfaceDefinitions: BundleResources Output: obj/Debug/ibtool/Main.storyboardc/MainMenu.nib obj/Debug/ibtool/Main.storyboardc/XfG-lQ-9wD-view-m2S-Jp-Qdl.nib obj/Debug/ibtool/Main.storyboardc/NSWindowController-B8D-0N-5wS.nib obj/Debug/ibtool/Main.storyboardc/Info.plist OutputManifests Output: obj/Debug/ibtool-manifests/Main.storyboardc Target _CoreCompileColladaAssets: Skipping target "_CoreCompileColladaAssets" because it has no inputs. Target GenerateTargetFrameworkMonikerAttribute: Skipping target "GenerateTargetFrameworkMonikerAttribute" because all output files are up-to-date with respect to the input files. Target CoreCompile: /Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/msbuild/Current/bin/Roslyn/csc.exe /noconfig /nowarn:1701,1702,2008 /nostdlib+ /errorreport:prompt /warn:4 /define:__UNIFIED__;__MACOS__;DEBUG /errorendlocation /preferreduilang:en-US /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/mscorlib.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/System.Core.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/System.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/Facades/System.Drawing.Common.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/Xamarin.Mac.dll /debug+ /debug:full /optimize- /out:obj/Debug/44.exe /target:exe /utf8output /langversion:7.3 Main.cs AppDelegate.cs ViewController.cs ViewController.designer.cs "obj/Debug/Xamarin.Mac,Version=v2.0.AssemblyAttributes.cs" Using shared compilation with compiler from directory: /Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/msbuild/Current/bin/Roslyn Target CopyFilesToOutputDirectory: Copying file from "/Users/sergey/work/44/44/obj/Debug/44.exe" to "/Users/sergey/work/44/44/bin/Debug/44.exe". 44 -> /Users/sergey/work/44/44/bin/Debug/44.exe Copying file from "/Users/sergey/work/44/44/obj/Debug/44.pdb" to "/Users/sergey/work/44/44/bin/Debug/44.pdb". Target _DetectSigningIdentity: Detected signing identity: Bundle Id: com.companyname.x44 App Id: com.companyname.x44 Target _CopyContentToBundle: Skipping target "_CopyContentToBundle" because all output files are up-to-date with respect to the input files. Target _CompileAppManifest: Skipping target "_CompileAppManifest" because all output files are up-to-date with respect to the input files. Target _CompileToNative: /Library/Frameworks/Xamarin.Mac.framework/Versions/Current/bin/mmp @/Users/sergey/work/44/44/obj/Debug/response-file.rsp Build succeeded. 0 Warning(s) 0 Error(s) Time Elapsed 00:00:06.53 ========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ========== Build successful.

Example Project (If Possible)

44.zip

rolfbjarne commented 4 years ago

This can be reproduced with a simpler sample:

var array1 = new NSArray ();
var array2 = NSArray.FromNSObjects (new NSObject [0]);
Console.WriteLine ("Array 1: {0}", array1.Handle);
Console.WriteLine ("Array 2: {0}", array2.Handle);
Console.WriteLine ("Array1 == Array2: {0}", object.ReferenceEquals (array1, array2));
array2.Dispose ();
Console.WriteLine ("Array 1: {0}", array1.Handle);
Console.WriteLine ("Array 2: {0}", array2.Handle);

this prints:

Array 1: 140735586813024
Array 2: 140735586813024
Array1 == Array2: True
Array 1: 0
Array 2: 0

this is because Objective-C has a singleton for an empty NSArray, and re-uses that singleton.

What happens here:

We create a new managed instance of an NSArray. The underlying Objective-C instance is that singleton.
We call NSArray.FromNSObjects, which will create the native NSArray instance (and getting the singleton), and then check if we have an existing managed instance for that native NSArray instance (which we do: the NSArray we created in step 1), and in that case, return that instance.
Then we call Dispose on array2 (which is also the exact same managed instance as array1), effectively disposing array1 as well.

@snechaev is this behavior causing a problem for you? From what I can tell it shouldn't be harmful in any way, although I can certainly see that it looks very weird.

snechaev commented 4 years ago

Hmm, I thinking before about the some cache/singleton for empty NSArray (the thought were based on the same Handles for different NSArray objects), but my experiments with swift do not confirmed this. It's sad, but looks like I made mistake somewhere in swift.

So, since it's cocoa-level singleton trick, then the behaviour is totally clear now.

As per your question: this behaviour inducted the crash in my app in the following use-after-free scenario.

I have following classes (simplified example). The LargeObject contains some data, for which I want to call Dispose manually and do not wait the GC. The set of data for LargeObjects is provided from the external source and may be empty.

        public class LargeObject: NSObject, IDisposable
        {
                 ...
        }

        public class Container : NSObject, IDisposable
        {
            [Export("objects")]
            NSArray<LargeObject> Objects { get; set; }
            public Container(LargeObject[] objects)
            {
                Objects = NSArray<LargeObject>.FromNSObjects(objects);
            }

            protected override void Dispose(bool disposing)
            {
                if(disposing)
                {
                    foreach (var obj in Objects)
                    {
                        obj.Dispose();
                    }
                }
                base.Dispose(disposing);
            }
        }

So, the crash is in following scenario:

I create Container object with empty dataset - it leads to create an empty NSArray
Somewhere in the other window of the app I perform the operation with ArrayController as described in first post.
then I call Dispose on Container object. And it throws NullReferenceException when trying to enumerate Objects array (of course the Objects itself is not null at the moment, but it's handle=0).

Of course all works fine if the dataset is not empty or if user do not open the window with mentioned NSArrayController.

In my situation I just switched to NSMutableArray.

rolfbjarne commented 4 years ago

Thanks a lot for the explanation, I now see how this ended up causing problems for you.

I understand that using an NSMutableArray is a viable workaround for you, and that you don't need this fixed, is that correct? I can't see an easy fix for this bug, which means it'll probably be some time until we can get around to fixing it.

snechaev commented 4 years ago

Yes, the current workaround with NSMutableArray is good, so the fix for this issue is not vitally-important for me.

xamarin / xamarin-macios