Fix value type caching (#932)

Fix value type caching

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;
@@ -743,10 +744,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);
@@ -760,8 +761,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()
         {
@@ -2565,8 +2566,8 @@ public void TestEventSourceCaching()
             GC.WaitForPendingFinalizers();
             Assert.False(weakClassInstance.TryGetTarget(out _));
             Assert.False(weakEventHandlerClass.TryGetTarget(out _));
-        }
-
+        }
+
         class EventHandlerClass
         {
             private readonly Action eventCalled;
@@ -2627,5 +2628,49 @@ private void TestSupportedOSPlatformWarnings()
             WarningStatic.WarningEvent += (object s, Int32 v) => { }; // warning CA1416
         }
 #endif
+
+        [Fact]
+        private async Task TestPnpPropertiesAsync()
+        {
+            var requestedDeviceProperties = new List<string>()
+                {
+                    "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();
+        }
     }
 }

src/WinRT.Runtime/ComWrappersSupport.cs

@@ -280,10 +280,11 @@ private static Func<IInspectable, object> 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<Func<IInspectable, object>>(
-                Expression.Convert(Expression.Property(createInterfaceInstanceExpression, "Value"), typeof(object)), parms).Compile();
+                        typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType)));
+
+            var getValueExpression = Expression.Convert(Expression.Property(createInterfaceInstanceExpression, "Value"), typeof(object));
+            var setValueInWrapperexpression = Expression.New(typeof(ValueTypeWrapper).GetConstructor(new[] { typeof(object) }), getValueExpression);
+            return Expression.Lambda<Func<IInspectable, object>>(setValueInWrapperexpression, parms).Compile();
         }
 
         private static Func<IInspectable, object> CreateArrayFactory(Type implementationType)
@@ -296,8 +297,9 @@ private static Func<IInspectable, object> CreateArrayFactory(Type implementation
                     Expression.Call(parms[0],
                         typeof(IInspectable).GetMethod(nameof(IInspectable.As)).MakeGenericMethod(vftblType)));
 
-            return Expression.Lambda<Func<IInspectable, object>>(
-                Expression.Property(createInterfaceInstanceExpression, "Value"), parms).Compile();
+            var getValueExpression = Expression.Property(createInterfaceInstanceExpression, "Value");
+            var setValueInWrapperexpression = Expression.New(typeof(ValueTypeWrapper).GetConstructor(new[] { typeof(object) }), getValueExpression);
+            return Expression.Lambda<Func<IInspectable, object>>(setValueInWrapperexpression, parms).Compile();
         }
 
         internal static Func<IInspectable, object> CreateTypedRcwFactory(string runtimeClassName)
@@ -726,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;
+            }
+        }
     }
 }

src/WinRT.Runtime/ComWrappersSupport.net5.cs

@@ -92,24 +92,25 @@ private static T CreateRcwForComObject<T>(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
             // native pointers. We unwrap here to ensure that the user-experience is expected
             // and consumers get a string object for a Windows.Foundation.IReference<String>.
             // 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();
-            }
-
             return rcw switch
             {
-                ABI.System.Nullable<string> ns => (T)(object)ns.Value,
+                ABI.System.Nullable<string> ns => (T)(object)ns.Value,
                 ABI.System.Nullable<Type> nt => (T)(object)nt.Value,
+                ValueTypeWrapper vt => (T) vt.Value,
                 T castRcw => castRcw,
                 _ when tryUseCache => CreateRcwForComObject<T>(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)))

src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs

@@ -74,7 +74,8 @@ public static T CreateRcwForComObject<T>(IntPtr ptr)
             {
                 ABI.System.Nullable<string> ns => (T)(object)ns.Value,
                 ABI.System.Nullable<Type> nt => (T)(object)nt.Value,
-                _ => (T)rcw
+                ValueTypeWrapper vt => (T)vt.Value,
+                _ => (T)rcw
             };
         }