diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index bed4b2b282f3..91cf68615f2c 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -156,8 +156,19 @@ <_MaciOSStampDirectory>$(IntermediateOutputPath)stamp\ + + + + + + false + true + + + + true diff --git a/src/Foundation/NSObject2.cs b/src/Foundation/NSObject2.cs index b29708b2635f..a4d8cef08771 100644 --- a/src/Foundation/NSObject2.cs +++ b/src/Foundation/NSObject2.cs @@ -991,6 +991,26 @@ internal void ClearHandle () handle = NativeHandle.Zero; } + // This option is changed by setting the DisposeTaggedPointers MSBuild property in the project file. + static bool? dispose_tagged_pointers; + static bool DisposeTaggedPointers { + 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) { @@ -998,7 +1018,37 @@ protected virtual void Dispose (bool disposing) return; disposed = true; - if (handle != NativeHandle.Zero) { + var isTaggedPointerThatShouldNotBeDisposed = false; + if (!DisposeTaggedPointers) { + /* Tagged pointers are 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; + } +#else + const ulong _OBJC_TAG_MASK = 1UL << 63; +#endif + + unchecked { + var ulongHandle = (ulong) (IntPtr) handle; + var isTaggedPointer = (ulongHandle & _OBJC_TAG_MASK) == _OBJC_TAG_MASK; + isTaggedPointerThatShouldNotBeDisposed = isTaggedPointer; + } + } + + if (!isTaggedPointerThatShouldNotBeDisposed && handle != NativeHandle.Zero) { if (disposing) { ReleaseManagedRef (); } else { diff --git a/src/ILLink.Substitutions.MacCatalyst.xml b/src/ILLink.Substitutions.MacCatalyst.xml index 26e6e7d9662f..4ad5457c9269 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 aaca42d023a4..db0e8f833bf8 100644 --- a/src/ILLink.Substitutions.ios.xml +++ b/src/ILLink.Substitutions.ios.xml @@ -1,5 +1,9 @@ + + + + diff --git a/src/ILLink.Substitutions.macOS.xml b/src/ILLink.Substitutions.macOS.xml index 1ad679b886f2..43c4a43a67e9 100644 --- a/src/ILLink.Substitutions.macOS.xml +++ b/src/ILLink.Substitutions.macOS.xml @@ -1,5 +1,9 @@ + + + + diff --git a/src/ILLink.Substitutions.tvos.xml b/src/ILLink.Substitutions.tvos.xml index 0c34d7b72853..50ea6b8b105b 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 84efc3c6314c..9699b6b685d6 100644 --- a/tests/common/shared-dotnet.mk +++ b/tests/common/shared-dotnet.mk @@ -83,7 +83,7 @@ endif PLATFORM_UPPERCASE:=$(shell echo $(PLATFORM) | tr 'a-z' 'A-Z') 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 000000000000..9196312041cb --- /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 000000000000..465fa5046e93 --- /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 000000000000..6b0e2c773180 --- /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 000000000000..110d078f4577 --- /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 000000000000..6affa45ff122 --- /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 000000000000..86d408734aa8 --- /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 000000000000..110d078f4577 --- /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 000000000000..a77287b9ba00 --- /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 000000000000..110d078f4577 --- /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 000000000000..7036e612089c --- /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 000000000000..7016a5ea615d --- /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 000000000000..bd487ddcd88d --- /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 000000000000..110d078f4577 --- /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 8354884182b2..cdb3fde67281 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -3727,6 +3727,31 @@ public void InvalidSupportedOSPlatformVersion (ApplePlatform platform, string ru 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 _); + } + // macOS doesn't support UseNativeHttpHandler / any of our native http handlers being the default http handler. [Test] [TestCase (ApplePlatform.MacCatalyst, "NSUrlSessionHandler", "true")] diff --git a/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.Shared.cs b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.Shared.cs new file mode 100644 index 000000000000..fb733cf5b354 --- /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 new file mode 100644 index 000000000000..e9a9096096a4 --- /dev/null +++ b/tests/monotouch-test/ObjCRuntime/TaggedPointerTest.cs @@ -0,0 +1,43 @@ +using System; +using System.Linq; +using System.Runtime.CompilerServices; + +using Foundation; + +using NUnit.Framework; + +namespace MonoTouchFixtures.ObjCRuntime { + + [TestFixture] + [Preserve (AllMembers = true)] + public partial class TaggedPointerTest { + + [Test] + public void TaggedPointersArentDisposed () + { +#if NET10_0_OR_GREATER + var taggedPointersDisposedByDefault = false; +#else + var taggedPointersDisposedByDefault = true; +#endif + Assert.AreEqual (taggedPointersDisposedByDefault, ThrowsObjectDisposedExceptions (), "Default behavior"); + } + + [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."); + } + } +}