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

[Runtime Error] MissingMethodException caused by using ToHashSet method #22

Closed
rodion-m opened this issue Jun 7, 2021 · 7 comments · Fixed by #23
Closed

[Runtime Error] MissingMethodException caused by using ToHashSet method #22

rodion-m opened this issue Jun 7, 2021 · 7 comments · Fixed by #23

Comments

@rodion-m
Copy link

rodion-m commented Jun 7, 2021

Today we crashed in too unpleasant bug in .NET Standard + Linq.Extras:

System.MissingMethodException Method not found: 'System.Collections.Generic.HashSet`1<!!0> Linq.Extras.XEnumerable.ToHashSet(System.Collections.Generic.IEnumerable`1<!!0>, System.Collections.Generic.IEqualityComparer`1<!!0>)'.

To reproduce it:

  1. Create a project NetStandartProject targeted to .netstandard2.0
  2. Add a method void SomeMethod(int[] a) => a.ToHashSet();
  3. Create a second project NetCoreProject targeted to .netcoreapp2.1 or higher
  4. Add NetStandartProject into NetCoreProject dependencies
  5. Call SomeMethod from NetCoreProject
  6. Get System.MissingMethodException

Here is a complete example solution: https://github.com/rodion-m/LinqExtrasRuntimeException. Just run NetCoreProject.

I suggest to delete all methods like ToHashSet from .netstandart target and let the developer write new HashSet(a) instead of a.ToHashSet() than crash in runtime with MissingMethodException. I think you just should make .netstandart version of Linq.Extras equal to .netcore version of Linq.Extras.

@thomaslevesque
Copy link
Owner

Hi @rodion-m,

I think I see what the problem is...
NetStandard2Project references the netstandard2.0 version of Linq.Extras, which contains the ToHashSet method (since ToHashSet isn't in netstandard2.0). It's compiled against that method of that specific assembly.
But NetCoreProject sees the more appropriate netcoreapp2.0 version of Linq.Extras, so it picks it up, and it's this version that is copied to the output folder. Unfortunately it doesn't contain the ToHashSet method (since ToHashSet is already in netcoreapp2.0).
So eventually the code in NetStandard2Project executes against a version of Linq.Extras different from the one it was compiled against, hence the error.

I suggest to delete all methods like ToHashSet from .netstandart target

It would be the easiest option, but it would be a breaking change for anyone currently using it, and I try to avoid that as much as possible.

I had introduced new targets without ToHashSet in order to avoid an ambiguity at compile-time (#19), but I didn't anticipate that it would cause a runtime issue...

So now, if I remove these targets (or add the ToHashSet method in these targets), I reintroduce the ambiguity. If I don't, things break at runtime in scenarios like yours. Looks like I have a choice between two bad options...

In the long run, I guess I should probably remove those methods, since there will be less and less people targetings TFMs without them. But It'll have to be in a major version, since it's a breaking change.

@rodion-m
Copy link
Author

rodion-m commented Jun 9, 2021

So now, if I remove these targets (or add the ToHashSet method in these targets), I reintroduce the ambiguity. If I don't, things break at runtime in scenarios like yours. Looks like I have a choice between two bad options...

Yes, here is a real choice of lesser evil. And I think that runtime error - is the biggest evil. So you definitely should remove this bugged extension asap (it's very not good to get an exception where you least expect it). It's not a problem for developers to fix all calls to new HashSet(...).

@thomaslevesque
Copy link
Owner

@rodion-m I published a new release: 5.0.0-beta.1, in which I removed the problematic methods. I also changed the TFMs (removed old ones, added new ones).
Please let me know if it fixes the problem for you.

@rodion-m
Copy link
Author

@rodion-m I published a new release: 5.0.0-beta.1, in which I removed the problematic methods. I also changed the TFMs (removed old ones, added new ones).
Please let me know if it fixes the problem for you.

Great. Thank you!

@thomaslevesque
Copy link
Owner

thomaslevesque commented Oct 23, 2021

@rodion-m I realize I never actually published the stable 5.0.0 with these changes.
But maybe that's a good thing, because I found a new and better way to fix the issues with these methods.
Basically, instead of removing them in newer TFMs, I can leave them in place, but without the this modifier. I think this approach offers the best outcome:

  • I can keep the ToHashSet, Append and Prepend methods for users targeting older frameworks
  • They won't cause ambiguous call errors when the methods already exist in the project's target framework, since they won't be extension methods in newer TFMs
  • They will still be callable at runtime in scenarios like the one you described

In the end, I might not even need to publish a 5.x version at all, since there won't be breaking changes wrt 4.x (I'm undecided yet; maybe it's better to publish a 5.x anyway, because users who installed 5.0.0-beta.1 won't see a 4.x version as an upgrade)

@thomaslevesque
Copy link
Owner

@rodion-m FYI I just published 5.0.0, using the approach described above

@rodion-m
Copy link
Author

@rodion-m FYI I just published 5.0.0, using the approach described above

Great, thank you! I think we can close the issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants