Skip to content

Commit b1e8fe6

Browse files
jkotasjkotas
committed
[release/10.0] Consider all MemberInfo references for IsCollectible (dotnet#119869)
Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not. These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached. Fix dotnet#119697
1 parent 8ca6013 commit b1e8fe6

File tree

6 files changed

+58
-66
lines changed

6 files changed

+58
-66
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
173173
internal RuntimeType GetRuntimeType() { return m_declaringType; }
174174
internal RuntimeModule GetRuntimeModule() { return RuntimeTypeHandle.GetModule(m_declaringType); }
175175
internal RuntimeAssembly GetRuntimeAssembly() { return GetRuntimeModule().GetRuntimeAssembly(); }
176-
public override bool IsCollectible => m_declaringType.IsCollectible;
176+
public override bool IsCollectible => ReflectedTypeInternal.IsCollectible;
177177
#endregion
178178

179179
#region MethodBase Overrides

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeEventInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
129129
public override int MetadataToken => m_token;
130130
public override Module Module => GetRuntimeModule();
131131
internal RuntimeModule GetRuntimeModule() { return m_declaringType.GetRuntimeModule(); }
132-
public override bool IsCollectible => m_declaringType.IsCollectible;
132+
public override bool IsCollectible => ReflectedTypeInternal.IsCollectible;
133133
#endregion
134134

135135
#region EventInfo Overrides

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ internal RuntimeType GetDeclaringTypeInternal()
4545
public sealed override bool HasSameMetadataDefinitionAs(MemberInfo other) => HasSameMetadataDefinitionAsCore<RuntimeFieldInfo>(other);
4646

4747
public override Module Module => GetRuntimeModule();
48-
public override bool IsCollectible => m_declaringType.IsCollectible;
48+
public override bool IsCollectible => ReflectedTypeInternal.IsCollectible;
4949

5050
#endregion
5151

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,18 @@ internal void InvokePropertySetter(object? obj, BindingFlags invokeAttr, Binder?
319319

320320
public override ParameterInfo ReturnParameter => FetchReturnParameter();
321321

322-
public override bool IsCollectible => RuntimeMethodHandle.GetIsCollectible(new RuntimeMethodHandleInternal(m_handle)) != Interop.BOOL.FALSE;
322+
public override bool IsCollectible
323+
{
324+
get
325+
{
326+
if (ReflectedTypeInternal.IsCollectible)
327+
return true;
328+
329+
bool isCollectible = RuntimeMethodHandle.GetIsCollectible(new RuntimeMethodHandleInternal(m_handle)) != Interop.BOOL.FALSE;
330+
GC.KeepAlive(this); // We directly pass the native handle above - make sure this object stays alive for the call
331+
return isCollectible;
332+
}
333+
}
323334

324335
public override MethodInfo GetBaseDefinition()
325336
{

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
180180

181181
public override Module Module => GetRuntimeModule();
182182
internal RuntimeModule GetRuntimeModule() { return m_declaringType.GetRuntimeModule(); }
183-
public override bool IsCollectible => m_declaringType.IsCollectible;
183+
public override bool IsCollectible => ReflectedTypeInternal.IsCollectible;
184184

185185
public override bool Equals(object? obj) =>
186186
ReferenceEquals(this, obj) ||

‎src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/IsCollectibleTests.cs‎

Lines changed: 42 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -132,17 +132,10 @@ public void Assembly_IsCollectibleTrue_WhenUsingTestAssemblyLoadContext()
132132
}).Dispose();
133133
}
134134

135-
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
136-
[InlineData("MyField")]
137-
[InlineData("MyProperty")]
138-
[InlineData("MyMethod")]
139-
[InlineData("MyGenericMethod")]
140-
[InlineData("MyStaticMethod")]
141-
[InlineData("MyStaticField")]
142-
[InlineData("MyStaticGenericMethod")]
143-
public void MemberInfo_IsCollectibleFalse_WhenUsingAssemblyLoad(string memberName)
135+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
136+
public void MemberInfo_IsCollectibleFalse_WhenUsingAssemblyLoad()
144137
{
145-
RemoteExecutor.Invoke((marshalledName) =>
138+
RemoteExecutor.Invoke(() =>
146139
{
147140
Type t1 = Type.GetType(
148141
"TestCollectibleAssembly.MyTestClass, TestCollectibleAssembly, Version=1.0.0.0",
@@ -153,25 +146,18 @@ public void MemberInfo_IsCollectibleFalse_WhenUsingAssemblyLoad(string memberNam
153146

154147
Assert.NotNull(t1);
155148

156-
var member = t1.GetMember(marshalledName).FirstOrDefault();
157-
158-
Assert.NotNull(member);
149+
foreach (var member in t1.GetMembers(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic))
150+
{
151+
Assert.False(member.IsCollectible, member.ToString());
152+
}
159153

160-
Assert.False(member.IsCollectible);
161-
}, memberName).Dispose();
154+
}).Dispose();
162155
}
163156

164-
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
165-
[InlineData("MyStaticGenericField")]
166-
[InlineData("MyStaticField")]
167-
[InlineData("MyStaticGenericMethod")]
168-
[InlineData("MyStaticMethod")]
169-
[InlineData("MyGenericField")]
170-
[InlineData("MyGenericProperty")]
171-
[InlineData("MyGenericMethod")]
172-
public void MemberInfoGeneric_IsCollectibleFalse_WhenUsingAssemblyLoad(string memberName)
157+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
158+
public void MemberInfoGeneric_IsCollectibleFalse_WhenUsingAssemblyLoad()
173159
{
174-
RemoteExecutor.Invoke((marshalledName) =>
160+
RemoteExecutor.Invoke(() =>
175161
{
176162
Type t1 = Type.GetType(
177163
"TestCollectibleAssembly.MyGenericTestClass`1[System.Int32], TestCollectibleAssembly, Version=1.0.0.0",
@@ -182,25 +168,18 @@ public void MemberInfoGeneric_IsCollectibleFalse_WhenUsingAssemblyLoad(string me
182168

183169
Assert.NotNull(t1);
184170

185-
var member = t1.GetMember(marshalledName).FirstOrDefault();
186-
187-
Assert.NotNull(member);
171+
foreach (var member in t1.GetMembers(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic))
172+
{
173+
Assert.False(member.IsCollectible, member.ToString());
174+
}
188175

189-
Assert.False(member.IsCollectible);
190-
}, memberName).Dispose();
176+
}).Dispose();
191177
}
192178

193-
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
194-
[InlineData("MyField")]
195-
[InlineData("MyProperty")]
196-
[InlineData("MyMethod")]
197-
[InlineData("MyGenericMethod")]
198-
[InlineData("MyStaticMethod")]
199-
[InlineData("MyStaticField")]
200-
[InlineData("MyStaticGenericMethod")]
201-
public void MemberInfo_IsCollectibleTrue_WhenUsingAssemblyLoadContext(string memberName)
179+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
180+
public void MemberInfo_IsCollectibleTrue_WhenUsingAssemblyLoadContext()
202181
{
203-
RemoteExecutor.Invoke((marshalledName) =>
182+
RemoteExecutor.Invoke(() =>
204183
{
205184
AssemblyLoadContext alc = new TestAssemblyLoadContext();
206185

@@ -213,25 +192,18 @@ public void MemberInfo_IsCollectibleTrue_WhenUsingAssemblyLoadContext(string mem
213192

214193
Assert.NotNull(t1);
215194

216-
var member = t1.GetMember(marshalledName).FirstOrDefault();
217-
218-
Assert.NotNull(member);
195+
foreach (var member in t1.GetMembers(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic))
196+
{
197+
Assert.True(member.IsCollectible, member.ToString());
198+
}
219199

220-
Assert.True(member.IsCollectible);
221-
}, memberName).Dispose();
200+
}).Dispose();
222201
}
223202

224-
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
225-
[InlineData("MyStaticGenericField")]
226-
[InlineData("MyStaticField")]
227-
[InlineData("MyStaticGenericMethod")]
228-
[InlineData("MyStaticMethod")]
229-
[InlineData("MyGenericField")]
230-
[InlineData("MyGenericProperty")]
231-
[InlineData("MyGenericMethod")]
232-
public void MemberInfoGeneric_IsCollectibleTrue_WhenUsingAssemblyLoadContext(string memberName)
203+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
204+
public void MemberInfoGeneric_IsCollectibleTrue_WhenUsingAssemblyLoadContext()
233205
{
234-
RemoteExecutor.Invoke((marshalledName) =>
206+
RemoteExecutor.Invoke(() =>
235207
{
236208
AssemblyLoadContext alc = new TestAssemblyLoadContext();
237209

@@ -244,12 +216,12 @@ public void MemberInfoGeneric_IsCollectibleTrue_WhenUsingAssemblyLoadContext(str
244216

245217
Assert.NotNull(t1);
246218

247-
var member = t1.GetMember(marshalledName).FirstOrDefault();
219+
foreach (var member in t1.GetMembers(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic))
220+
{
221+
Assert.True(member.IsCollectible, member.ToString());
222+
}
248223

249-
Assert.NotNull(member);
250-
251-
Assert.True(member.IsCollectible);
252-
}, memberName).Dispose();
224+
}).Dispose();
253225
}
254226

255227
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
@@ -268,7 +240,16 @@ public void GenericWithCollectibleTypeParameter_IsCollectibleTrue_WhenUsingAssem
268240

269241
Assert.NotNull(t1);
270242

271-
Assert.True(t1.IsCollectible);
243+
foreach (var member in t1.GetMembers(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic))
244+
{
245+
if (member is Type)
246+
{
247+
continue;
248+
}
249+
250+
Assert.True(member.IsCollectible, member.ToString());
251+
}
252+
272253
}).Dispose();
273254
}
274255
}

0 commit comments

Comments
 (0)