From 953b9656316de1de95560032045645ee9b553b31 Mon Sep 17 00:00:00 2001 From: Ujjwal Chadha <66971578+ujjwalchadha@users.noreply.github.com> Date: Mon, 26 Jul 2021 18:05:16 -0700 Subject: [PATCH 1/3] Fix value type caching --- .../UnitTest/TestComponentCSharp_Tests.cs | 52 ++++++++++++++++++- src/WinRT.Runtime/ComWrappersSupport.cs | 10 ++-- src/WinRT.Runtime/ComWrappersSupport.net5.cs | 7 ++- .../ComWrappersSupport.netstandard2.0.cs | 11 ++-- 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs index 44f2515f1..229fcd14c 100644 --- a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs +++ b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs @@ -28,7 +28,8 @@ using Windows.Security.Cryptography; using Windows.Security.Cryptography.Core; using System.Reflection; - +using Windows.Devices.Enumeration.Pnp; + #if NET5_0 using WeakRefNS = System; #else @@ -2517,5 +2518,54 @@ private void TestSupportedOSPlatformWarnings() WarningStatic.WarningEvent += (object s, Int32 v) => { }; // warning CA1416 } #endif + + [Fact] + private void TestPnpProperties() + { + TestPnpPropertiesAsync().Wait(10000); + } + + private async Task TestPnpPropertiesAsync() + { + var requestedDeviceProperties = new List() + { + "System.Devices.ClassGuid", + "System.Devices.ContainerId", + "System.Devices.DeviceHasProblem", + "System.Devices.DeviceInstanceId", + "System.Devices.Parent", + "System.Devices.Present", + "System.ItemNameDisplay", + "System.Devices.Children", + }; + var devicefilter = "System.Devices.Present:System.StructuredQueryType.Boolean#True"; + var presentDevices = (await PnpObject.FindAllAsync(PnpObjectType.Device, requestedDeviceProperties, devicefilter).AsTask().ConfigureAwait(false)).Select(pnpObject => { + var prop = pnpObject.Properties; + // Iterating through each key is necessary for this test even though we do not use each key directly + // This makes it more probable for a native pointer to get repeated and a value type to be cached and seen again. + foreach (var key in prop.Keys) + { + var val = prop[key]; + if (key == "System.Devices.ContainerId" && val != null) + { + var val4 = pnpObject.Properties[key]; + if (val is not Guid || val4 is not Guid) + { + throw new Exception("Incorrect value type Guid"); + } + } + if (key == "System.Devices.Parent" && val != null) + { + var val4 = pnpObject.Properties[key]; + if (val is not string || val4 is not string) + { + throw new Exception("Incorrect value type string"); + } + } + + } + return pnpObject; + }).ToList(); + } } } diff --git a/src/WinRT.Runtime/ComWrappersSupport.cs b/src/WinRT.Runtime/ComWrappersSupport.cs index c8e705abd..948fd9afd 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.cs @@ -280,10 +280,9 @@ private static Func CreateNullableTFactory(Type implementa ParameterExpression[] parms = new[] { Expression.Parameter(typeof(IInspectable), "inspectable") }; var createInterfaceInstanceExpression = Expression.New(helperType.GetConstructor(new[] { typeof(ObjectReference<>).MakeGenericType(vftblType) }), Expression.Call(parms[0], - typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - - return Expression.Lambda>( - Expression.Convert(Expression.Property(createInterfaceInstanceExpression, "Value"), typeof(object)), parms).Compile(); + typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); + + return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); } private static Func CreateArrayFactory(Type implementationType) @@ -296,8 +295,7 @@ private static Func CreateArrayFactory(Type implementation Expression.Call(parms[0], typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - return Expression.Lambda>( - Expression.Property(createInterfaceInstanceExpression, "Value"), parms).Compile(); + return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); } internal static Func CreateTypedRcwFactory(string runtimeClassName) diff --git a/src/WinRT.Runtime/ComWrappersSupport.net5.cs b/src/WinRT.Runtime/ComWrappersSupport.net5.cs index 9e23bf3c4..e80d43ba2 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.net5.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.net5.cs @@ -106,10 +106,13 @@ private static T CreateRcwForComObject(IntPtr ptr, bool tryUseCache) winrtObj.Resurrect(); } + if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) + { + return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); + } + return rcw switch { - ABI.System.Nullable ns => (T)(object)ns.Value, - ABI.System.Nullable nt => (T)(object)nt.Value, T castRcw => castRcw, _ when tryUseCache => CreateRcwForComObject(ptr, false), _ => throw new ArgumentException(string.Format("Unable to create a wrapper object. The WinRT object {0} has type {1} which cannot be assigned to type {2}", ptr, rcw.GetType(), typeof(T))) diff --git a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs index d85cd79bf..a36381cdf 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs @@ -70,12 +70,11 @@ public static T CreateRcwForComObject(IntPtr ptr) // and consumers get a string object for a Windows.Foundation.IReference. // We need to do the same thing for System.Type because there can be multiple MUX.Interop.TypeName's // for a single System.Type. - return rcw switch - { - ABI.System.Nullable ns => (T)(object)ns.Value, - ABI.System.Nullable nt => (T)(object)nt.Value, - _ => (T)rcw - }; + if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) + { + return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); + } + return (T) rcw; } public static bool TryUnwrapObject(object o, out IObjectReference objRef) From 64636769a31528d81004edd96202bb8367b55a0c Mon Sep 17 00:00:00 2001 From: Ujjwal Chadha <66971578+ujjwalchadha@users.noreply.github.com> Date: Mon, 26 Jul 2021 18:05:16 -0700 Subject: [PATCH 2/3] Fix value type caching --- .../UnitTest/TestComponentCSharp_Tests.cs | 68 ++++++++++++++++--- src/WinRT.Runtime/ComWrappersSupport.cs | 10 ++- src/WinRT.Runtime/ComWrappersSupport.net5.cs | 7 +- .../ComWrappersSupport.netstandard2.0.cs | 11 ++- 4 files changed, 73 insertions(+), 23 deletions(-) diff --git a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs index cee10cd4e..0afa8e429 100644 --- a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs +++ b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs @@ -28,6 +28,7 @@ using Windows.Security.Cryptography; using Windows.Security.Cryptography.Core; using System.Reflection; +using Windows.Devices.Enumeration.Pnp; #if NET using WeakRefNS = System; @@ -49,11 +50,11 @@ public class TestCSharp public TestCSharp() { TestObject = new Class(); - } - + } + // Test a fix for a bug in Mono.Cecil that was affecting the IIDOptimizer when it encountered long class names - [Fact] + [Fact] public void TestLongClassNameEventSource() { bool flag = false; @@ -717,10 +718,10 @@ public void TestCustomProjections() Assert.Equal("name", propertyName); bool eventCalled = false; - TestObject.CanExecuteChanged += (object sender, EventArgs e) => - { - eventCalled = true; - }; + TestObject.CanExecuteChanged += (object sender, EventArgs e) => + { + eventCalled = true; + }; TestObject.RaiseCanExecuteChanged(); Assert.True(eventCalled); @@ -734,8 +735,8 @@ public void TestCustomProjections() // Ensure robustness with bad runtime class names (parsing errors, type not found, etc) var badRuntimeClassName = Class.BadRuntimeClassName; Assert.NotNull(badRuntimeClassName); - } - + } + [Fact] public void TestKeyValuePair() { @@ -2601,5 +2602,54 @@ private void TestSupportedOSPlatformWarnings() WarningStatic.WarningEvent += (object s, Int32 v) => { }; // warning CA1416 } #endif + + [Fact] + private void TestPnpProperties() + { + TestPnpPropertiesAsync().Wait(10000); + } + + private async Task TestPnpPropertiesAsync() + { + var requestedDeviceProperties = new List() + { + "System.Devices.ClassGuid", + "System.Devices.ContainerId", + "System.Devices.DeviceHasProblem", + "System.Devices.DeviceInstanceId", + "System.Devices.Parent", + "System.Devices.Present", + "System.ItemNameDisplay", + "System.Devices.Children", + }; + var devicefilter = "System.Devices.Present:System.StructuredQueryType.Boolean#True"; + var presentDevices = (await PnpObject.FindAllAsync(PnpObjectType.Device, requestedDeviceProperties, devicefilter).AsTask().ConfigureAwait(false)).Select(pnpObject => { + var prop = pnpObject.Properties; + // Iterating through each key is necessary for this test even though we do not use each key directly + // This makes it more probable for a native pointer to get repeated and a value type to be cached and seen again. + foreach (var key in prop.Keys) + { + var val = prop[key]; + if (key == "System.Devices.ContainerId" && val != null) + { + var val4 = pnpObject.Properties[key]; + if (val is not Guid || val4 is not Guid) + { + throw new Exception("Incorrect value type Guid"); + } + } + if (key == "System.Devices.Parent" && val != null) + { + var val4 = pnpObject.Properties[key]; + if (val is not string || val4 is not string) + { + throw new Exception("Incorrect value type string"); + } + } + + } + return pnpObject; + }).ToList(); + } } } diff --git a/src/WinRT.Runtime/ComWrappersSupport.cs b/src/WinRT.Runtime/ComWrappersSupport.cs index 51ab7715a..02209f0f8 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.cs @@ -280,10 +280,9 @@ private static Func CreateNullableTFactory(Type implementa ParameterExpression[] parms = new[] { Expression.Parameter(typeof(IInspectable), "inspectable") }; var createInterfaceInstanceExpression = Expression.New(helperType.GetConstructor(new[] { typeof(ObjectReference<>).MakeGenericType(vftblType) }), Expression.Call(parms[0], - typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - - return Expression.Lambda>( - Expression.Convert(Expression.Property(createInterfaceInstanceExpression, "Value"), typeof(object)), parms).Compile(); + typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); + + return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); } private static Func CreateArrayFactory(Type implementationType) @@ -296,8 +295,7 @@ private static Func CreateArrayFactory(Type implementation Expression.Call(parms[0], typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - return Expression.Lambda>( - Expression.Property(createInterfaceInstanceExpression, "Value"), parms).Compile(); + return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); } internal static Func CreateTypedRcwFactory(string runtimeClassName) diff --git a/src/WinRT.Runtime/ComWrappersSupport.net5.cs b/src/WinRT.Runtime/ComWrappersSupport.net5.cs index 9e23bf3c4..e80d43ba2 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.net5.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.net5.cs @@ -106,10 +106,13 @@ private static T CreateRcwForComObject(IntPtr ptr, bool tryUseCache) winrtObj.Resurrect(); } + if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) + { + return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); + } + return rcw switch { - ABI.System.Nullable ns => (T)(object)ns.Value, - ABI.System.Nullable nt => (T)(object)nt.Value, T castRcw => castRcw, _ when tryUseCache => CreateRcwForComObject(ptr, false), _ => throw new ArgumentException(string.Format("Unable to create a wrapper object. The WinRT object {0} has type {1} which cannot be assigned to type {2}", ptr, rcw.GetType(), typeof(T))) diff --git a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs index d85cd79bf..a36381cdf 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs @@ -70,12 +70,11 @@ public static T CreateRcwForComObject(IntPtr ptr) // and consumers get a string object for a Windows.Foundation.IReference. // We need to do the same thing for System.Type because there can be multiple MUX.Interop.TypeName's // for a single System.Type. - return rcw switch - { - ABI.System.Nullable ns => (T)(object)ns.Value, - ABI.System.Nullable nt => (T)(object)nt.Value, - _ => (T)rcw - }; + if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) + { + return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); + } + return (T) rcw; } public static bool TryUnwrapObject(object o, out IObjectReference objRef) From ea42222e39248207158f96b7f0ddec8db8c4cee6 Mon Sep 17 00:00:00 2001 From: UJJWAL CHADHA Date: Thu, 29 Jul 2021 22:36:56 -0700 Subject: [PATCH 3/3] Added value type wrapper --- .../UnitTest/TestComponentCSharp_Tests.cs | 5 ---- src/WinRT.Runtime/ComWrappersSupport.cs | 23 +++++++++++++++++-- src/WinRT.Runtime/ComWrappersSupport.net5.cs | 22 ++++++++---------- .../ComWrappersSupport.netstandard2.0.cs | 12 ++++++---- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs index d51ad38ea..c3d05092b 100644 --- a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs +++ b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs @@ -2604,11 +2604,6 @@ private void TestSupportedOSPlatformWarnings() #endif [Fact] - private void TestPnpProperties() - { - TestPnpPropertiesAsync().Wait(10000); - } - private async Task TestPnpPropertiesAsync() { var requestedDeviceProperties = new List() diff --git a/src/WinRT.Runtime/ComWrappersSupport.cs b/src/WinRT.Runtime/ComWrappersSupport.cs index 02209f0f8..396c5ff25 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.cs @@ -282,7 +282,9 @@ private static Func CreateNullableTFactory(Type implementa Expression.Call(parms[0], typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); + var getValueExpression = Expression.Convert(Expression.Property(createInterfaceInstanceExpression, "Value"), typeof(object)); + var setValueInWrapperexpression = Expression.New(typeof(ValueTypeWrapper).GetConstructor(new[] { typeof(object) }), getValueExpression); + return Expression.Lambda>(setValueInWrapperexpression, parms).Compile(); } private static Func CreateArrayFactory(Type implementationType) @@ -295,7 +297,9 @@ private static Func CreateArrayFactory(Type implementation Expression.Call(parms[0], typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType))); - return Expression.Lambda>(createInterfaceInstanceExpression, parms).Compile(); + var getValueExpression = Expression.Property(createInterfaceInstanceExpression, "Value"); + var setValueInWrapperexpression = Expression.New(typeof(ValueTypeWrapper).GetConstructor(new[] { typeof(object) }), getValueExpression); + return Expression.Lambda>(setValueInWrapperexpression, parms).Compile(); } internal static Func CreateTypedRcwFactory(string runtimeClassName) @@ -724,5 +728,20 @@ internal InspectableInfo(Type type, Guid[] iids) } } + + internal class ValueTypeWrapper + { + private readonly object value; + + public ValueTypeWrapper(object value) + { + this.value = value; + } + + public object Value + { + get => this.value; + } + } } } \ No newline at end of file diff --git a/src/WinRT.Runtime/ComWrappersSupport.net5.cs b/src/WinRT.Runtime/ComWrappersSupport.net5.cs index e80d43ba2..c37df17e0 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.net5.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.net5.cs @@ -92,6 +92,13 @@ private static T CreateRcwForComObject(IntPtr ptr, bool tryUseCache) var flags = tryUseCache ? CreateObjectFlags.TrackerObject : CreateObjectFlags.TrackerObject | CreateObjectFlags.UniqueInstance; var rcw = ComWrappers.GetOrCreateObjectForComInstance(ptr, flags); CreateRCWType.Value = null; + + // Resurrect IWinRTObject's disposed IObjectReferences, if necessary + if (rcw is IWinRTObject winrtObj) + { + winrtObj.Resurrect(); + } + // Because .NET will de-duplicate strings and WinRT doesn't, // our RCW factory returns a wrapper of our string instance. // This ensures that ComWrappers never sees the same managed object for two different @@ -99,20 +106,11 @@ private static T CreateRcwForComObject(IntPtr ptr, bool tryUseCache) // and consumers get a string object for a Windows.Foundation.IReference. // We need to do the same thing for System.Type because there can be multiple WUX.Interop.TypeName's // for a single System.Type. - - // Resurrect IWinRTObject's disposed IObjectReferences, if necessary - if (rcw is IWinRTObject winrtObj) - { - winrtObj.Resurrect(); - } - - if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) - { - return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); - } - return rcw switch { + ABI.System.Nullable ns => (T)(object)ns.Value, + ABI.System.Nullable nt => (T)(object)nt.Value, + ValueTypeWrapper vt => (T) vt.Value, T castRcw => castRcw, _ when tryUseCache => CreateRcwForComObject(ptr, false), _ => throw new ArgumentException(string.Format("Unable to create a wrapper object. The WinRT object {0} has type {1} which cannot be assigned to type {2}", ptr, rcw.GetType(), typeof(T))) diff --git a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs index a36381cdf..705a3f1e3 100644 --- a/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs +++ b/src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs @@ -70,11 +70,13 @@ public static T CreateRcwForComObject(IntPtr ptr) // and consumers get a string object for a Windows.Foundation.IReference. // We need to do the same thing for System.Type because there can be multiple MUX.Interop.TypeName's // for a single System.Type. - if (rcw.GetType().FullName.StartsWith("ABI.System.Nullable`1") || rcw.GetType().FullName.StartsWith("ABI.Windows.Foundation.IReferenceArray`1")) - { - return (T) rcw.GetType().GetProperty("Value").GetGetMethod().Invoke(rcw, null); - } - return (T) rcw; + return rcw switch + { + ABI.System.Nullable ns => (T)(object)ns.Value, + ABI.System.Nullable nt => (T)(object)nt.Value, + ValueTypeWrapper vt => (T)vt.Value, + _ => (T)rcw + }; } public static bool TryUnwrapObject(object o, out IObjectReference objRef)