You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
NullabilityInfoContext will throw IndexOutOfRangeException when calling Create method on certain Dictionary-valued properties in generic classes. Seems to happen when the TValue of the dictionary is a generic class with one type parameter coinciding with the type parameter of the class that defines the property.
Reproduction Steps
// .NET 6.0 LTS Console App project with default settings (nullability context enabled).
using System.Reflection;
Foo<int>? bar = new();
NullabilityInfoContext context = new();
PropertyInfo x = bar.GetType().GetProperty(nameof(bar.Bad))!;
// The next line will throw.
NullabilityInfo info = context.Create(x);
Console.WriteLine(info.ReadState);
public class Foo<T>
{
public Dictionary<int, List<T>>? Bad { get; set; }
public Dictionary<int, IEquatable<T>>? AlsoBad { get; set; }
public Dictionary<int, List<int>>? Good { get; set; }
public Dictionary<int, T>? AlsoGood { get; set; }
}
Expected behavior
Program should successfully terminate with Nullable displayed on the console.
Actual behavior
Line NullabilityInfo info = context.Create(x); throws a System.IndexOutOfRangeException.
at System.Reflection.NullabilityInfoContext.CheckGenericParameters(NullabilityInfo nullability, MemberInfo metaMember, Type metaType)
at System.Reflection.NullabilityInfoContext.TryLoadGenericMetaTypeNullability(MemberInfo memberInfo, NullabilityInfo nullability)
at System.Reflection.NullabilityInfoContext.GetNullabilityInfo(MemberInfo memberInfo, Type type, IList`1 customAttributes, Int32 index)
at System.Reflection.NullabilityInfoContext.GetNullabilityInfo(MemberInfo memberInfo, Type type, IList`1 customAttributes, Int32 index)
at System.Reflection.NullabilityInfoContext.Create(PropertyInfo propertyInfo)
Regression?
No response
Known Workarounds
No response
Configuration
This is a vanilla .NET 6.0 Long-term Support "Console App" project created in Visual Studio 2022 on a Windows 11 machine. Fails in both Debug and Release mode when targeting "Any CPU".
I'll give it a shot :^) However, I'm very new to creating PR and almost hopeless at git. So this will take me a while to read through all relevant instructions.
as for (int i = 0; i < genericArguments.Length && i < nullability.GenericTypeArguments.Length; i++) . However, I think it is not the right fix as nullability.GenericTypeArguments should account for same number of generic arguments as metadata.GetGenericArguments() so the checks will run for each nested argument as well, right?
@am11 I don't recall running into this. I'm surprised that the test cases don't cover this given that the repro seems fairly simple.
However, I think it is not the right fix as nullability.GenericTypeArguments should account for same number of generic arguments as metadata.GetGenericArguments()
Agreed. We need to figure out why these differ.
I'd be happy to take this if @ropufu doesn't end up wanting to.
Thank you for the nudge. I wasn't for a while: I could not follow the Contributing/Workflow guidelines to set up debugging for the issue in conjunction with System.Private.CoreLib back then... and I still can't.
If there is a deadline for me to aim at--that would be helpful (assuming that leaves you folks enough time to crack it if I fail). And if you have other references (like a minimal example/walkthrough how to set up a debuggable project in VS that targets the preview .NET) that would be most welcome. My apologies for not speaking up the first time I could not do it.
@buyaa-n Thank you so much for your help. Right to the point. Saved me a lot of frustration and pain. Thank you!
I think I managed to get the thing fixed. Reflection tests look good; I'll try to get the proper submission done in an hour or two. My only concern is that I am not so fluent in the NullabilityInfoContext class as to understand all of the potential ramifications of my change (although I don't think there should be any).
Activity
ropufu commentedon Apr 24, 2022
Here are some more cases that will fail, this time without referencing lists or dictionaries.
danmoseley commentedon Apr 24, 2022
Thanks for the report @ropufu . Any interest in debugging a bit, or offering a fix?
ropufu commentedon Apr 24, 2022
I'll give it a shot :^) However, I'm very new to creating PR and almost hopeless at git. So this will take me a while to read through all relevant instructions.
danmoseley commentedon Apr 24, 2022
Sure - take your time and let me know if you get stuck. There are pretty good workflow instructions though.
am11 commentedon Apr 25, 2022
Nice report, @ropufu. I can reproduce it with net7.0 as well. A quick fix is to change this line:
runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs
Line 462 in a9f9745
as
for (int i = 0; i < genericArguments.Length && i < nullability.GenericTypeArguments.Length; i++). However, I think it is not the right fix asnullability.GenericTypeArgumentsshould account for same number of generic arguments asmetadata.GetGenericArguments()so the checks will run for each nested argument as well, right?@madelson, do you remember seeing this case while working on #64143?
madelson commentedon Apr 25, 2022
@am11 I don't recall running into this. I'm surprised that the test cases don't cover this given that the repro seems fairly simple.
Agreed. We need to figure out why these differ.
I'd be happy to take this if @ropufu doesn't end up wanting to.
buyaa-n commentedon Jul 6, 2022
Hello @ropufu? Are you still working on it? It better to be fixed within 7.0 preview 7 and not many days left
ropufu commentedon Jul 6, 2022
Thank you for the nudge. I wasn't for a while: I could not follow the Contributing/Workflow guidelines to set up debugging for the issue in conjunction with System.Private.CoreLib back then... and I still can't.
If there is a deadline for me to aim at--that would be helpful (assuming that leaves you folks enough time to crack it if I fail). And if you have other references (like a minimal example/walkthrough how to set up a debuggable project in VS that targets the preview .NET) that would be most welcome. My apologies for not speaking up the first time I could not do it.
buyaa-n commentedon Jul 6, 2022
After building
clr+libs+libs.testsas mentioned in the Contributing/Workflow guidelines you could be able to add tests into https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs and debug from thatSystem.Runtimewhich references code inSystem.Private.CoreLibThe preview 7 deadline is next Monday, so if you could do the fix within this week
ropufu commentedon Jul 8, 2022
@buyaa-n Thank you so much for your help. Right to the point. Saved me a lot of frustration and pain. Thank you!
I think I managed to get the thing fixed. Reflection tests look good; I'll try to get the proper submission done in an hour or two. My only concern is that I am not so fluent in the
NullabilityInfoContextclass as to understand all of the potential ramifications of my change (although I don't think there should be any).Moved call to TryLoadGenericMetaTypeNullability