diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index fda2bdb0b9..68536264ea 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -167,8 +167,19 @@ <_MaciOSStampDirectory>$(IntermediateOutputPath)stamp\ + + + + + + false + true + + + + true @@ -573,9 +584,6 @@ <_ValidateObjectPointers Condition="'$(_ValidateObjectPointers)' == ''">false - - <_DisposeTaggedPointers Condition="'$(_DisposeTaggedPointers)' == ''">true - <_CustomLinkerOptions> AreAnyAssembliesTrimmed=$(_AreAnyAssembliesTrimmed) AssemblyName=$(AssemblyName).dll @@ -690,7 +698,6 @@ - dispose_tagged_pointers; - set => dispose_tagged_pointers = value; + get { + if (!dispose_tagged_pointers.HasValue) { + if (AppContext.TryGetSwitch ("Foundation.NSObject.DisposeTaggedPointers", out var dtp)) { + dispose_tagged_pointers = dtp; + } else { + // The default logic here must match how we set the default value for the DisposeTaggedPointers MSBuild property. +#if NET10_0_OR_GREATER + dispose_tagged_pointers = false; +#else + dispose_tagged_pointers = true; +#endif + } + } + return dispose_tagged_pointers.Value; + } } protected virtual void Dispose (bool disposing) @@ -1011,34 +1024,37 @@ namespace Foundation { return; disposed = true; - /* Tagged pointer is limited to 64bit, which is all we support anyway. - * - * The tagged pointer bit is: - * - * Arm64: most significant bit - * Simulators (both on arm64 and x64 desktops): most significant bit - * Desktop/x64 (macOS + Mac Catalyst): least significant bit - * Ref: https://github.com/apple-oss-distributions/objc4/blob/89543e2c0f67d38ca5211cea33f42c51500287d5/runtime/objc-internal.h#L603-L672 - */ + var isTaggedPointerThatShouldNotBeDisposed = false; + if (!DisposeTaggedPointers) { + /* Tagged pointer is limited to 64bit, which is all we support anyway. + * + * The bit that identifies if a pointer is a tagged pointer is: + * + * Arm64 (everywhere): most significant bit + * Simulators (both on arm64 and x64 desktops): most significant bit + * Desktop/x64 (macOS + Mac Catalyst): least significant bit + * + * Ref: https://github.com/apple-oss-distributions/objc4/blob/89543e2c0f67d38ca5211cea33f42c51500287d5/runtime/objc-internal.h#L603-L672 + */ #if __MACOS__ || __MACCATALYST__ - ulong _OBJC_TAG_MASK; - if (Runtime.IsARM64CallingConvention) { - _OBJC_TAG_MASK = 1UL << 63; - } else { - _OBJC_TAG_MASK = 1UL; - } + ulong _OBJC_TAG_MASK; + if (Runtime.IsARM64CallingConvention) { + _OBJC_TAG_MASK = 1UL << 63; + } else { + _OBJC_TAG_MASK = 1UL; + } #else - const ulong _OBJC_TAG_MASK = 1UL << 63; + const ulong _OBJC_TAG_MASK = 1UL << 63; #endif - bool isTaggedPointer; - unchecked { - var ulongHandle = (ulong) (IntPtr) handle; - isTaggedPointer = (ulongHandle & _OBJC_TAG_MASK) == _OBJC_TAG_MASK; + unchecked { + var ulongHandle = (ulong) (IntPtr) handle; + var isTaggedPointer = (ulongHandle & _OBJC_TAG_MASK) == _OBJC_TAG_MASK; + isTaggedPointerThatShouldNotBeDisposed = isTaggedPointer; + } } - if (!DisposeTaggedPointers && isTaggedPointer) { - // don't dispose tagged pointers. + if (isTaggedPointerThatShouldNotBeDisposed) { FreeData (); // still need to do this though. } else if (handle != NativeHandle.Zero) { if (disposing) { diff --git a/src/ILLink.Substitutions.MacCatalyst.xml b/src/ILLink.Substitutions.MacCatalyst.xml index f0839f2359..1d4aeb0021 100644 --- a/src/ILLink.Substitutions.MacCatalyst.xml +++ b/src/ILLink.Substitutions.MacCatalyst.xml @@ -1,5 +1,9 @@ + + + + diff --git a/src/ILLink.Substitutions.ios.xml b/src/ILLink.Substitutions.ios.xml index 22124d05e5..19ee305559 100644 --- a/src/ILLink.Substitutions.ios.xml +++ b/src/ILLink.Substitutions.ios.xml @@ -1,5 +1,9 @@ + + + + diff --git a/src/ILLink.Substitutions.tvos.xml b/src/ILLink.Substitutions.tvos.xml index 19d660dbb2..7ca0ab2049 100644 --- a/src/ILLink.Substitutions.tvos.xml +++ b/src/ILLink.Substitutions.tvos.xml @@ -1,5 +1,9 @@ + + + + diff --git a/tests/common/shared-dotnet.mk b/tests/common/shared-dotnet.mk index 91caa4161a..13d922493a 100644 --- a/tests/common/shared-dotnet.mk +++ b/tests/common/shared-dotnet.mk @@ -73,7 +73,7 @@ PLATFORM=$(shell basename "$(CURDIR)") endif ifneq ($(RUNTIMEIDENTIFIERS)$(RUNTIMEIDENTIFIER),) -$(error "Don't set RUNTIMEIDENTIFIER or RUNTIMEIDENTIFIERS, set RID instead") +$(error "Don't set RUNTIMEIDENTIFIER or RUNTIMEIDENTIFIERS, set RID instead (RUNTIMEIDENTIFIER=$(RUNTIMEIDENTIFIER), RUNTIMEIDENTIFIERS=$(RUNTIMEIDENTIFIERS))") endif ifeq ($(RID),) diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/AppDelegate.cs b/tests/dotnet/DisposeTaggedPointersTestApp/AppDelegate.cs new file mode 100644 index 0000000000..46feac1c75 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/AppDelegate.cs @@ -0,0 +1,93 @@ +using System; +using System.Runtime.InteropServices; + +using Foundation; + +namespace DisposeTaggedPointersTestApp { + public class Program { + static int Main (string [] args) + { + var testCaseString = Environment.GetEnvironmentVariable ("TEST_CASE"); + if (string.IsNullOrEmpty (testCaseString)) { + Console.WriteLine ($"The environment variable TEST_CASE wasn't set."); + return 2; + } +#if NET10_0_OR_GREATER + var taggedPointersDisposedByDefault = false; +#else + var taggedPointersDisposedByDefault = true; +#endif + switch (testCaseString) { + case "DisableWithAppContext": + AppContext.SetSwitch ("Foundation.NSObject.DisposeTaggedPointers", false); + if (ThrowsObjectDisposedExceptions ()) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected ObjectDisposedException when DisposeTaggedPointers=false"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + case "EnableWithAppContext": + AppContext.SetSwitch ("Foundation.NSObject.DisposeTaggedPointers", true); + if (!ThrowsObjectDisposedExceptions ()) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected lack of ObjectDisposedException when DisposeTaggedPointers=true"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + case "DisableWithMSBuildPropertyTrimmed": + if (ThrowsObjectDisposedExceptions ()) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected ObjectDisposedException when DisposeTaggedPointers=false"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + case "EnableWithMSBuildPropertyTrimmed": + if (!ThrowsObjectDisposedExceptions ()) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected lack of ObjectDisposedException when DisposeTaggedPointers=true"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + case "DisableWithMSBuildPropertyUntrimmed": { + var throws = ThrowsObjectDisposedExceptions (); + if (throws == taggedPointersDisposedByDefault) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected ObjectDisposedException when DisposeTaggedPointers=false"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + } + case "EnableWithMSBuildPropertyUntrimmed": { + var throws = ThrowsObjectDisposedExceptions (); + if (throws != taggedPointersDisposedByDefault) { + Console.WriteLine ($"❌ {testCaseString}: Failure: unexpected lack of ObjectDisposedException when DisposeTaggedPointers=true"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + } + case "Default": { + var throws = ThrowsObjectDisposedExceptions (); + if (throws != taggedPointersDisposedByDefault) { + Console.WriteLine ($"❌ {testCaseString}: Failure: Expected ObjectDisposedException: {taggedPointersDisposedByDefault} Threw ObjectDisposedException: {throws}"); + return 1; + } + + Console.WriteLine ($"✅ {testCaseString}: Success"); + return 0; + } + default: + Console.WriteLine ($"❌ Unknown test case: {testCaseString }"); + return 2; + } + } + + static bool ThrowsObjectDisposedExceptions () => MonoTouchFixtures.ObjCRuntime.TaggedPointerTest.ThrowsObjectDisposedExceptions (); + } +} diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/Directory.Build.props b/tests/dotnet/DisposeTaggedPointersTestApp/Directory.Build.props new file mode 100644 index 0000000000..465fa5046e --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/Directory.Build.props @@ -0,0 +1,7 @@ + + + + false + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/DisposeTaggedPointersTestApp.csproj b/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/DisposeTaggedPointersTestApp.csproj new file mode 100644 index 0000000000..6b0e2c7731 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/DisposeTaggedPointersTestApp.csproj @@ -0,0 +1,7 @@ + + + + net$(BundledNETCoreAppTargetFrameworkVersion)-maccatalyst + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/Makefile b/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/Makefile new file mode 100644 index 0000000000..110d078f45 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/MacCatalyst/Makefile @@ -0,0 +1 @@ +include ../shared.mk diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/Makefile b/tests/dotnet/DisposeTaggedPointersTestApp/Makefile new file mode 100644 index 0000000000..6affa45ff1 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/Makefile @@ -0,0 +1,2 @@ +TOP=../../.. +include $(TOP)/tests/common/shared-dotnet-test.mk diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/iOS/DisposeTaggedPointersTestApp.csproj b/tests/dotnet/DisposeTaggedPointersTestApp/iOS/DisposeTaggedPointersTestApp.csproj new file mode 100644 index 0000000000..86d408734a --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/iOS/DisposeTaggedPointersTestApp.csproj @@ -0,0 +1,7 @@ + + + + net$(BundledNETCoreAppTargetFrameworkVersion)-ios + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/iOS/Makefile b/tests/dotnet/DisposeTaggedPointersTestApp/iOS/Makefile new file mode 100644 index 0000000000..110d078f45 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/iOS/Makefile @@ -0,0 +1 @@ +include ../shared.mk diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/macOS/DisposeTaggedPointersTestApp.csproj b/tests/dotnet/DisposeTaggedPointersTestApp/macOS/DisposeTaggedPointersTestApp.csproj new file mode 100644 index 0000000000..a77287b9ba --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/macOS/DisposeTaggedPointersTestApp.csproj @@ -0,0 +1,7 @@ + + + + net$(BundledNETCoreAppTargetFrameworkVersion)-macos + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/macOS/Makefile b/tests/dotnet/DisposeTaggedPointersTestApp/macOS/Makefile new file mode 100644 index 0000000000..110d078f45 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/macOS/Makefile @@ -0,0 +1 @@ +include ../shared.mk diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/shared.csproj b/tests/dotnet/DisposeTaggedPointersTestApp/shared.csproj new file mode 100644 index 0000000000..7036e61208 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/shared.csproj @@ -0,0 +1,18 @@ + + + + Exe + + DisposeTaggedPointersTestApp + com.xamarin.disposetaggedpointerstestapp + + true + + + + + + + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/shared.mk b/tests/dotnet/DisposeTaggedPointersTestApp/shared.mk new file mode 100644 index 0000000000..7016a5ea61 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/shared.mk @@ -0,0 +1,44 @@ +TOP=../../../.. +TESTNAME=DisposeTaggedPointersTestApp +include $(TOP)/tests/common/shared-dotnet.mk + +export RUNTIMEIDENTIFIER= +export RUNTIMEIDENTIFIERS= + +.stamp-restore: + $(Q) $(DOTNET) restore + $(Q) touch $@ + +define Test +clean-$(2): + $(Q) rm -f .stamp-build-$(2) + $(Q) rm -rf bin/$(1) obj/$(1) +CLEAN_TARGETS+=clean-$(2) + +.stamp-build-$(2): .stamp-restore + $(MAKE) build CONFIG=$(1) BUILD_ARGUMENTS="/tl:off $(3)" + $(Q) touch $$@ +BUILD_TARGETS+=.stamp-build-$(2) + +run-$(2): export TEST_CASE=$(1) +run-$(2): .stamp-build-$(2) + $(MAKE) run-bare CONFIG=$(1) +RUN_TARGETS+=run-$(2) + +test-$(2): run-$(2) +endef + +$(eval $(call Test,Default,default)) +$(eval $(call Test,DisableWithAppContext,disablewithappcontext)) +$(eval $(call Test,EnableWithAppContext,enablewithappcontext)) +$(eval $(call Test,DisableWithMSBuildPropertyUntrimmed,disablewithmsbuildpropertyuntrimmed,/p:DisposeTaggedPointers=false /p:_LinkMode=None)) +$(eval $(call Test,EnableWithMSBuildPropertyUntrimmed,enablewithmsbuildpropertyuntrimmed,/p:DisposeTaggedPointers=true /p:_LinkMode=None)) +$(eval $(call Test,DisableWithMSBuildPropertyTrimmed,disablewithmsbuildpropertytrimmed,/p:DisposeTaggedPointers=false /p:_LinkMode=SdkOnly)) +$(eval $(call Test,EnableWithMSBuildPropertyTrimmed,enablewithmsbuildpropertytrimmed,/p:DisposeTaggedPointers=true /p:_LinkMode=SdkOnly)) + +build-all: $(BUILD_TARGETS) +clean-all:: $(CLEAN_TARGETS) + $(Q) rm -f .stamp-* *.binlog +test-all run-tests run-all run: $(RUN_TARGETS) + +.PHONY: $(TEST_CASES) diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/DisposeTaggedPointersTestApp.csproj b/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/DisposeTaggedPointersTestApp.csproj new file mode 100644 index 0000000000..bd487ddcd8 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/DisposeTaggedPointersTestApp.csproj @@ -0,0 +1,7 @@ + + + + net$(BundledNETCoreAppTargetFrameworkVersion)-tvos + + + diff --git a/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/Makefile b/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/Makefile new file mode 100644 index 0000000000..110d078f45 --- /dev/null +++ b/tests/dotnet/DisposeTaggedPointersTestApp/tvOS/Makefile @@ -0,0 +1 @@ +include ../shared.mk diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 5e881d167f..12f9e96fcc 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -2864,5 +2864,29 @@ namespace Xamarin.Tests { var errors = BinLog.GetBuildLogErrors (rv.BinLogPath).ToArray (); AssertErrorMessages (errors, $"The SupportedOSPlatformVersion value '{version}' in the project file is lower than the minimum value '{minVersion}'."); } + + [TestCase (ApplePlatform.MacOSX)] + [TestCase (ApplePlatform.MacCatalyst)] + public void DisposeTaggedPointersTest (ApplePlatform platform) + { + var project = "DisposeTaggedPointersTestApp"; + Configuration.IgnoreIfIgnoredPlatform (platform); + + var project_path = GetProjectPath (project, platform: platform); + Clean (project_path); + + var arguments = new string [] { + "-C", Path.GetDirectoryName (project_path)!, + "clean-all" + }; + AssertExecute ("make", arguments, out var _); + + arguments = new string [] { + "-C", Path.GetDirectoryName (project_path)!, + "test-all", + "-j", + }; + AssertExecute ("make", arguments, out var _); + } } } diff --git a/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.Shared.cs b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.Shared.cs new file mode 100644 index 0000000000..fb733cf5b3 --- /dev/null +++ b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.Shared.cs @@ -0,0 +1,53 @@ +using System; + +using Foundation; + +namespace MonoTouchFixtures.ObjCRuntime { + public partial class TaggedPointerTest { + public static bool ThrowsObjectDisposedExceptions () + { + try { + var notificationData = + """ + { + action = "action"; + aps = + { + category = "ee"; + "content-available" = dd; + "mutable-content" = cc; + sound = bb; + "thread-id" = "aa"; + }; + "em-account" = "a"; + "em-account-id" = "b"; + "em-body" = "c"; + "em-date" = "d"; + "em-from" = "e"; + "em-from-address" = "f"; + "em-notification" = g; + "em-notification-id" = h; + "em-subject" = "i"; + "gcm.message_id" = j; + "google.c.a.e" = k; + "google.c.fid" = l; + "google.c.sender.id" = m; + } + """; + + var data = NSData.FromString (notificationData); + var fmt = NSPropertyListFormat.OpenStep; + var userInfo = (NSMutableDictionary) NSPropertyListSerialization.PropertyListWithData (data, NSPropertyListReadOptions.Immutable, ref fmt, out var error); + var apsKey = new NSString ("aps"); + // Iteration here should not throw ObjectDisposedExceptions + foreach (var kv in userInfo) { + apsKey.Dispose (); + } + + return false; + } catch (ObjectDisposedException) { + return true; + } + } + } +} diff --git a/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.cs b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.cs index 988d5bf34c..e9a9096096 100644 --- a/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.cs +++ b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.cs @@ -1,4 +1,6 @@ using System; +using System.Linq; +using System.Runtime.CompilerServices; using Foundation; @@ -8,47 +10,34 @@ namespace MonoTouchFixtures.ObjCRuntime { [TestFixture] [Preserve (AllMembers = true)] - public class TaggedPointerTest { + public partial class TaggedPointerTest { [Test] public void TaggedPointersArentDisposed () { - var notificationData = - """ - { - action = "action"; - aps = - { - category = "ee"; - "content-available" = dd; - "mutable-content" = cc; - sound = bb; - "thread-id" = "aa"; - }; - "em-account" = "a"; - "em-account-id" = "b"; - "em-body" = "c"; - "em-date" = "d"; - "em-from" = "e"; - "em-from-address" = "f"; - "em-notification" = g; - "em-notification-id" = h; - "em-subject" = "i"; - "gcm.message_id" = j; - "google.c.a.e" = k; - "google.c.fid" = l; - "google.c.sender.id" = m; - } - """; +#if NET10_0_OR_GREATER + var taggedPointersDisposedByDefault = false; +#else + var taggedPointersDisposedByDefault = true; +#endif + Assert.AreEqual (taggedPointersDisposedByDefault, ThrowsObjectDisposedExceptions (), "Default behavior"); + } - var data = NSData.FromString (notificationData); - var fmt = NSPropertyListFormat.OpenStep; - var userInfo = (NSMutableDictionary) NSPropertyListSerialization.PropertyListWithData (data, NSPropertyListReadOptions.Immutable, ref fmt, out var error); - var apsKey = new NSString ("aps"); - // Iteration here should not throw ObjectDisposedExceptions - foreach (var kv in userInfo) { - apsKey.Dispose (); + [Test] + public void MemoryUsage () + { + var taggedStringValue = "a"; + var objA = new NSString (taggedStringValue); + var objB = new NSString (taggedStringValue); + Assert.AreEqual (objA.Handle, objB.Handle, "Pointer equality for tagged pointers"); + + var cwt = new ConditionalWeakTable (); + var count = 1000; + for (var i = 0; i < count; i++) { + cwt.Add (i.ToString (), new NSString (taggedStringValue)); } + GC.Collect (); + Assert.That (cwt.Count (), Is.LessThan (count), "At least some objects should have gotten garbage collected."); } } }