runtime: Unused generic type parameter should not cause loader failure
Consider the following code:
static class Program
{
static void Main()
{
var m = new M();
}
}
struct N<T> { }
struct M { public N<M> E; }
This code will compile with C# 6.0 and is legal according to the CLI spec. The type definition is recursive but the layout of the struct is not because the field involved here is an empty struct. The CLR is unable to handle this though and fails at runtime with a TypeLoadException:
‘M:Program.Main’ failed: Could not load type ‘M’ from assembly ‘ConsoleApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null’. System.TypeLoadException: Could not load type ‘M’ from assembly ‘ConsoleApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null’. at Program.Main()
See also dotnet/runtime#5479
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 95
- Comments: 50 (18 by maintainers)
Commits related to this issue
- Potential fix for #6924 - Take advantage of work done a few years ago to simplify the interaction with the interop subsystem - In the problematic case, simulate loads with two different types as insta... — committed to davidwrighton/runtime by davidwrighton a year ago
- Use similar types for self-referential generics instead of the exact canonical type (#83995) - Take advantage of work done a few years ago to simplify the interaction with the interop subsystem - In... — committed to dotnet/runtime by davidwrighton a year ago
To give an update on the demand to fix this one year after @davidwrighton’s last message, the number of 👍s has more than doubled from 24 to 60, making this issue by far the most upvoted open or closed bug of this repo, and three duplicate issues have been opened since that time.
Closing since there doesnt seem to be any particular reasons to be fix in the near term.
This codebase has buckets of unit tests, but also buckets of emergent behavior that are customer visible such as exception ordering, assembly load ordering etc, and lots of extremely obscure corner cases. Unfortunately, the code that this impacts (field layout) is some of the most sensitive code in the runtime to odd, subtle, and obscure failure modes which are difficult to test. (We have thousands of field layout tests, and each release we discover new ways our testing was inadequate. It is extremely frustrating.) Each release we look at this bug, and try to determine if its important enough to counterbalance the fairly likely destabilizing effect that a fix would imply, and the time available to do type system work, and we haven’t been able to justify the work.
However, we do look at the issue upvote count and as @karelz notes the upvote count is up to 24 which is fairly high for an issue in this repo which indicates there is real customer demand for us to fix this, so later this year we will look at scheduling this for the next release, and what the estimated cost will be. We may be able to look at this for .NET 7. We may not.
We’ve had to work around this issue multiple times in Roslyn. Did you need separate reports each time we hit this to realize that it is a persistent problem?
Apparently the “full-fledged” CLR is the only runtime where it doesn’t work. Mono and experimental CoreRT have no issues with running this code.
I have encountered this as I attempted to “compress” a tree of nodes. In essence I have code like this:
I’m frustrated that this has yet to be fixed after literal years of it being a thing. Much longer and this issue alone will have been open for a decade…
Whether this issue will be resolved or not, the user experience is currently really bad. We just ran into this issue because a colleague wanted to keep a LUT inside a value type using
ImmutableArray<T>(company coding rules say no mutable static members soT[]would be forbidden).All I get is a
TypeLoadExceptionwith no hint as to what is going wrong. When it first showed up it was in the middle of a massive file that was affected by several different source generators and it was very hard to know what it was causing the problem.I even tried using
dotnet-tracein hopes of getting a better idea, but it just crashed and produced a truncated trace.I eventually built runtime to debug it myself and found the exception thrown in
clsload.cpp:3451So even here there is not indication as to what is actually going wrong.
If the runtime will not support this then Roslyn should not allow the code to compile in the first place. Internally we should be able to add this case to our analyzers so people are not caught off guard in the future, but we shouldn’t be having to to do that ourselves.
No idea - maybe @davidwrighton or @jkotas can comment?
I still believe it is not serious enough to prioritize it. If you have a fix, I think we would be open to a contribution (if it is not overly complex, it is high-quality and not breaking). General rule is Future = maybe one day.
Reading this discussion is pretty disheartening.
I ran into this bug working on a struct to provide easy traversal and stitching functionality for objects that have linked list style
nextIndexfields embedded within them. I am attempting to represent parent-child-sibling-sibling-sibling… relationships between objects (structs in my current case) of the same type; which causes me to have a, for example,struct Linker<Entry>member inside ofstruct Entry(causing this bug to manifest).Linker<T>in this case carries a head index, a tail index, and a pointer to a function that extracts theref nextindex fromref T(which is why it requires the generic parameter). Can I work around this bug? Certainly. Will the results be half as readable to whoever inherits this code? Unlikely.It looks like this issue is specific to value types; I could convert my struct to a reference type and I’d imagine this problem goes away; but then what’s the point of packing my memory to avoid cache misses?
I understand that it may appear that this bug is not being triggered by many, however I’d argue that those who are triggering it are the type of users who are convincing their businesses to take chances on R&D efforts or unusual optimizations. The type of users I’d imagine you were targeting when you added
Span<T>, newrefsupport, function pointers, etc. Users that build cool new stuff with your cool new stuff. I hope that you will increase the priority of fixing this bug specifically, and this class of bug in general. Please help us avoid unnecessary gotchas like this one. Please help us deliver our best with your tools.@mangod9 Is it really a good approach to simply close issues regarding bugs, just because you don’t plan to fix them in the near time? Especially considering that even the Roslyn team had to workaround this bug several times. Personally I’m still waiting and hoping for this to be fixed, and having an open issue is a nice way to at least track it.
I understand that it can be discouraging to have eternally open bug issues… But simply closing them without any resolution just seems weird and the wrong way to handle things.
edit: One of my most frustrating situations in recent times is to look up a bug… Only to find an issue closed due to no activity, while the bug still remains to the present day. Depending on the project this happens fairly frequently. It saddens me to see .NET go the same way.
Can the exception message at least be changed to indicate the cause of the failure? Having to prune code bit-by-bit to find an obscure type-system limitation is a real time waster.
@karelz I understand this seems to be relativly small impact but it is very fundamental in nature, a very foundational capability. In my case I’m working on a C# rewrite of a custom memory allocator that sits on top of an application’s unmanaged memory space. Recent advances to C# in areas like type constraints and the
refkeyword have made this possible. However this bug is a solid roadblock because we have a generic struct typeSmartPointer<T>which supports lists and trees that cannot be implemented because code compiles but fails at runtime.The struct type
SmartPointer<T>when being used in a list, appears as a member field in a struct typeTthat can “point” to another instance of aT, this is where the recursive reference comes up. For reasons I can’t go into here, we cannot use a conventionalunsafepointer.(Just to be clear these struct instances are not in managed memory but allocated from unmanaged area of a process’s addess space using the custom allocator).
Is there any possibility the bug could be given to a competent intern to see if they can do anything with it? I dont care about it being in 3.0, but currently its going nowhere and this could continue for years more if Microsoft continue to treat it as low importance.
Leaving it unfixed for year after year could also make it harder to fix because the CLR may get refactored or restructured in ways that make the fix more difficult. This feature really is a basic expectation.
Id love to look at it myself even but will not pretend I have the internals knowledge or experience to actually deliver.
Suggest changing the title from “Unused generic type parameter …” to “Self-referencing generic type parameter …”, as it’s not related to whether the type parameter is used or unused - see @verelpode’s example above, and also https://github.com/dotnet/coreclr/issues/20220, which now seems to be a duplicate of this.
@karelz - OK so you wrote the above around two years ago and there’s still absolutely no movement it seems on this. This just strikes me as odd, the system is failing to run code that compiles cleanly, we have valid, legal C# yet get assembly load failures - surely this can be given more attention???
FWIW, this is an issue we hit repeatedly, particularly in the context of generated C# code, where it is often not easy or feasible to work around (changing output of the code generator wholesale to avoid this problem would be API breaking or have unacceptable knock-on effects, detecting and avoiding on a case-by-case basis would lead to inconsistent or incompatible APIs in some cases).
This issue becomes less academic and much more significant when one begins to leverage recently added support for generic pointers, the
unmanagedgeneric constraint andref.Here is a concrete and realistic example (which comes from actual code I’ve developed) https://github.com/dotnet/roslyn/issues/35324.
(sorry, closed by accident - hotkeys 😦)
Still no progress on this bug. I can only assume that a few users are raising this, because a truly fundamental failure to process completely legal generated code strikes me as something that would get a little more attention.
That should probably be a new issue so that it has a new thumbs up counter with people who’re blocked by this. We can wait until someone runs into it.
The counter is used to prioritize the bug against any other type loader work - i.e. there’s a finite number of people who can fix such bug and working on fixing the bug means other things will not be worked on. It’s zero sum.
Should this be reopened since there are still cases not covered by the fix, such as the one mentioned here?
The backtrace from 1765d037c80f241bf6b391ed84c20cd8deb3dd32 runtime's debug build looks like this.
The exception is thrown from https://github.com/dotnet/runtime/blob/69e114c1abf91241a0eeecf1ecceab4711b8aa62/src/coreclr/vm/clsload.cpp#L3777-L3781
I just bumped into this issue. My workaround was to create an array.
This has been a known issue in the CLR typesystem since generics were added many years ago. The general problem is that the field type of fields is computed at type load time during initial creation of the type data structures, and that would have to change as part of fixing this issue. In general, it is believed that changes to logic in this system are somewhat high in risk, and without a clear justification for risky work, we can’t justify the work. There is a related similar issue with static fields, where some ECMA permitted static fields are not loadable by the runtime.
To the point that @Korporal was making that leaving this issue around will make it harder to fix in the future, unfortunately that isn’t really the case. The reasons for this being difficult in the CLR have been part of the type loader design since the late 90’s when the runtime was first implemented before generics were even designed in the first place, and so I don’t believe we are in significant risk of making it a more difficult fix.
I decided, seeing as it’s been forever and likely gonna take forever more, that I’d put careful thought into an analyzer of my own design to help deal with this issue.
https://github.com/sunkin351/GenericStructCyclesAnalyzer
Using this would deal with any user experience issues revolving around this. The analyzer itself is incredibly simple, single file and catches every situation I could think of.
I would like to propose adding it to the existing suite of .NET Analyzers, but I’m not sure it would stand up to their guidelines. Maybe some of you could check that for me.
@pengweiqhca That code contains a GC hole and isn’t safe to use.
Unsafe.AsPointerwill cause the GC to lose track of the reference, leading to memory corruption. This is how you can avoid the GC hole:The general rule is that you should almost never convert a managed
refto an unmanaged pointer withoutfixed, unless therefis referring to unmanaged or already-pinned memory.I resolved.
Just encountered this issue trying to specifically avoid recursion by boxing the reference 😅
(Btw, I know there are other ways to do this… Just wanted to point out that I too was a bit puzzled when I encountered the issue)
Just encountered this with
ReadOnlyMemory<Self>. Very odd thatSelf[]is allowed but(ReadOnly)Memory<Self>breaks.@karelz I see there’s no movement on this, is anyone in a position to say that this will or won’t be addressed and if so when approx it might be fixed?
It’s been around for three and a half year though!
People might reply, “True but nobody needs an empty struct in practice”, therefore here is an example that throws TypeLoadException with a non-empty struct. i.e. a more severe example of the failure.