Disposing immutable objects can lead to unpredictable behavior when Objective-C runtime does object pooling

[filipnavara]

Consider the following code:

string notificationData =
    """
    {
        action = "show_pushed_mail";
        aps =
        {
            alert =
            {
                body = "Compared Exchange Rates are out of tolerance in number of 115.";
                subtitle = "navara@emclient.com";
                title = "eM Client Licensing";
            };
            category = "PUSHED_MAIL";
            "content-available" = 1;
            "mutable-content" = 1;
            sound = default;
            "thread-id" = "mail_cf73f3bd-1cf9-437c-bc70-4b6bd2b5f7e4";
        };
        "em-account" = "navara@emclient.com";
        "em-account-id" = "cf73f3bd-1cf9-437c-bc70-4b6bd2b5f7e4";
        "em-body" = "";
        "em-date" = "2024-10-09T01:30:01.0000000Z";
        "em-from" = "eM Client Licensing";
        "em-from-address" = "licensing@emclient.com";
        "em-message-path" = "{\n \"Mailbox\": \"INBOX\",\n \"UIDVALIDITY\": \"1117940911\",\n \"UID\": \"317970\",\n \"Message-ID\": \"6bb2b08b259b4ed1b558c737a49f5c39@emclient.com\"\n}";
        "em-notification" = Mixed;
        "em-notification-id" = 5801afc0ceb977f379adc152333e0d25554a98d4;
        "em-subject" = "Compared Exchange Rates are out of tolerance in number of 115.";
        "gcm.message_id" = 1728437407395630;
        "google.c.a.e" = 1;
        "google.c.fid" = eEhP3hWX1U8DuSB9hb3siN;
        "google.c.sender.id" = 417058856903;
    }
    """;

var d = NSData.FromString(notificationData);
NSPropertyListFormat fmt = NSPropertyListFormat.OpenStep;
var userInfo = (NSMutableDictionary)NSPropertyListSerialization.PropertyListWithData(d, ref fmt, out var error);

var apsKey = new NSString("aps");
var data = userInfo
    .Where(kv =>
    {
        var r = !kv.Key.ToString().Equals("aps", StringComparison.OrdinalIgnoreCase);
        apsKey.Dispose();
        return r;
    })
    .ToDictionary(kv => kv.Key.ToString(), kv => kv.Value.ToString() ?? string.Empty);

It will reliably crash with ObjectDisposedException which may be quite unexpected. In fact, this is a very reduced example of a problem that was happening in a multi-threaded application where it was even less obvious.

Why does it crash?

• We create the NSString object representing the string aps.
• When Objective-C enumerates the dictionary keys it reuses the same object, ie. same handle, when enumerating the aps key, and that in turns maps to the same managed NSString object.
• Calling Dispose on the NSString causes the managed representation to be invalid and next enumeration of the same key will crash with ObjectDisposedException.

What can we do about it?

Changing the behavior to remove disposed objects from the handle->managed object mapping seems dangerous. (would not help anyway in the original multi-threaded scenario)

Make the Dispose on immutable poolable classes like NSString a no-op? Make an analyzer that warns when someone tries to dispose a NSString instance (would flag the obvious error but not if someone disposes it as NSObject variable)?

Thoughts?

[rolfbjarne]

This is a tricky problem, and we've run into different variations on it over the years.

Make the Dispose on immutable poolable classes like NSString a no-op? Make an analyzer that warns when someone tries to dispose a NSString instance (would flag the obvious error but not if someone disposes it as NSObject variable)?

The problem with this is that I can certainly envision someone wanting to dispose at least some NSString instances (they can be arbitrarily big, so you might want to dispose big ones).

One idea I just had would be to just skip disposing tagged pointers, but the main problem with this approach is that there's no reliable way to detect tagged pointers (Apple can change their implementation at any time). Just doing a string comparison on the Objective-C class (NSTaggedPointerString), but it's a bit scary to have correctness depend on this (if Apple changed this class's name, apps could randomly start throwing ObjectDisposedException...)

[filipnavara]

Presumably, if this is affecting all tagged objects, we can differentiate by the lowest (x64) / highest (arm64) bit == 1 and disallow Dispose on those (disallow as in make it a no-op). I don't like the fact that it's dependency on internal implementation detail but it's probably just as reliable as checking for NSTaggedPointerString and it solves the same issue for NSNumber (and NSColor?).

I'm not sure if there are any other cases of reused objects aside from the tagged ones.

[rolfbjarne]

Presumably, if this is affecting all tagged objects, we can differentiate by the lowest (x64) / highest (arm64) bit == 1 and disallow Dispose on those (disallow as in make it a no-op). I don't like the fact that it's dependency on internal implementation detail

That internal implementation detail changed in iOS 14: "Let's explore one more change coming this year. A change to the tagged pointer format on arm64." - https://developer.apple.com/videos/play/wwdc2020/10163/

Which means we'll have to detect (and not apply the tagged pointer logic) when running on iOS 13 or earlier (our min iOS is right now iOS 12.2 in .NET 9). Alternatively not enable it when an app's min OS version is <14.

[filipnavara]

I think the part about the tagging logic that changed doesn't affect the "is tagged" bit, only the other ones which we do not rely on.