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 for issue where plugins that reference types from other assemblies cause Pipeline to display the "Invalid / Missing importer" error #6566

Open
wants to merge 5 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@timesscar
Copy link

commented Nov 28, 2018

Fix for issue where plugins that reference types from other assemblies cause Pipeline to crash and/or display the "Invalid / Missing importer" error

Fix for issue where plugins that reference types from other assemblie…
…s cause Pipeline to crash and/or display the "Invalid / Missing importer" error

@timesscar timesscar changed the title Develop Fix for issue where plugins that reference types from other assemblies cause Pipeline to crash and/or display the "Invalid / Missing importer" error Nov 28, 2018

@Jjagg

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

@timesscar Can you provide an explanation of what issue this fixes and how?

@timesscar

This comment has been minimized.

Copy link
Author

commented Nov 28, 2018

EDIT: Apologies, I realized that the issue in 6538 was not the same because the outright crash was a result of testing. The exception in this case is swallowed higher up the stack and just causes the plugin to not load. I opened issue #6567

Hello, it addresses a similar issue to #6538. The exception in my case is thrown at

var types = asm.GetTypes();
when using a pipeline plugin that references other assemblies. The technique used here won't work in this case because the assemblies end up in the app domain but dependencies are not resolvable because they are not in the same directory as pipeline. The technique in
var a = Assembly.Load(File.ReadAllBytes(path));
is the correct one in this case. This PR just splits the reflection between appdomain assemblies and plugin assemblies into two functions for clarity and causes the assemblies loaded from plugins to be ignored when loading from the appdomain.

Note, I do need to add an iteration on this to check to make sure the assembly hasn't been seen first before adding it to the hashset. EDIT: Done.

@timesscar timesscar changed the title Fix for issue where plugins that reference types from other assemblies cause Pipeline to crash and/or display the "Invalid / Missing importer" error Fix for issue where plugins that reference types from other assemblies cause Pipeline to display the "Invalid / Missing importer" error Nov 28, 2018

@Jjagg

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

cc @cra0zy

@cra0zy

This comment has been minimized.

Copy link
Member

commented Nov 29, 2018

Hmm, this should have been fixed with overriding the assembly resolver event: https://github.com/MonoGame/MonoGame/blob/develop/Tools/Pipeline/Common/PipelineTypes.cs#L209
(the event above only gets called when the reference cannot be resolved).

Can you provide an example content project that fails with MG 3.8?

@timesscar

This comment has been minimized.

Copy link
Author

commented Nov 29, 2018

Is there a way I can send you the bits? You'd need my assemblies too. Alternatively, I can invite you to my VSTS repo and you can build it directly.

EDIT: Just to note, the issue is because the assemblies are in the appdomain after they are loaded from the plugins. So the variable

if (string.IsNullOrEmpty(_currentAssemblyDirectory))
is not set anymore when they are encountered later.

@timesscar

This comment has been minimized.

Copy link
Author

commented Dec 1, 2018

Sorry for the iteration spam, but I reworked the resolver to fit into the pattern you already had better and fix an issue I encountered while testing.

@cra0zy

This comment has been minimized.

Copy link
Member

commented Dec 1, 2018

If I understood you correctly, assemblies get loaded correct the first time, but after that they are returned in new List<Assembly>(AppDomain.CurrentDomain.GetAssemblies()) and so the variable does not get set properly for resolving the 2nd time around?

@timesscar

This comment has been minimized.

Copy link
Author

commented Dec 1, 2018

Yes, that's correct. The issue I encountered that warranted the further updates was that the N level deep references cause the same issue. The latest iteration caches the requesting assembly so the children of the children (of the children etc) should be OK too.

@cra0zy

This comment has been minimized.

Copy link
Member

commented Dec 1, 2018

If thats the case, than the proper fix should be to properly unload all the assemblies we've loaded into appdomain before we try to reload.

Now that I think about it there is probably another bug with loading multiple projects that use the same reference, but different versions of it.

@timesscar

This comment has been minimized.

Copy link
Author

commented Dec 1, 2018

I thought about that, but there's extra complexity reloading the other types you need, not just the plugins. I thought caching the dll locations would be a reasonable middle ground.

KevinYeti pushed a commit to KevinYeti/MonoGame that referenced this pull request Feb 12, 2019

shenyi
Fix for issue where plugins that reference types from other assemblie…
…s cause Pipeline to display the "Invalid / Missing importer" error

MonoGame#6566
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.