Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix value type caching #932

Merged
merged 5 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 56 additions & 11 deletions src/Tests/UnitTest/TestComponentCSharp_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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()
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
}
29 changes: 23 additions & 6 deletions src/WinRT.Runtime/ComWrappersSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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;
}
}
}
}
17 changes: 9 additions & 8 deletions src/WinRT.Runtime/ComWrappersSupport.net5.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
3 changes: 2 additions & 1 deletion src/WinRT.Runtime/ComWrappersSupport.netstandard2.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}

Expand Down