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

Load assemblies into common appdomain/loadcontext #178

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

riverar
Copy link
Contributor

@riverar riverar commented Oct 27, 2019

.NET Core implementation of Assembly.Load creates a new AssemblyLoadContext, isolating assemblies and ultimately resulting in dependency load related failures (e.g. #177).

This change loads telemetry and Azure Storage assemblies into the default AppDomain or AssemblyLoadContext via Assembly.LoadFrom.

@HowardWolosky
Copy link
Member

Closing this out as there isn't any net benefit from making this specific change.
If there's a misunderstanding here, please let me know and we can reactivate and discuss further.

@riverar
Copy link
Contributor Author

riverar commented Apr 24, 2020

It's required if you have any plans to support PowerShell Core. If no plans, then agree.

@HowardWolosky
Copy link
Member

Ah, right. I forgot to re-read #177 for the context reminder. I never did the investigation to verify that this change doesn't break PS4. Once verified, this can get merged in. Thanks Rafael.

@HowardWolosky HowardWolosky reopened this Apr 24, 2020
@HowardWolosky
Copy link
Member

@riverar - I don't know how so much time passed on this, but I'm ready to get this merged-in. Ran into a problem with GitHub's web resolver UI, and it resolved a conflict in Telemetry.ps1 but changed the BOM type, which now makes the whole file look different where it should actually be completely unchanged. Any chance you can submit an update that just reverts the change to Telemetry.ps1 (as #201 removed assembly loading from that file).

@riverar riverar force-pushed the coreclr-alc-support branch from f5a4927 to f775a48 Compare August 24, 2020 01:47
@riverar
Copy link
Contributor Author

riverar commented Aug 24, 2020

@HowardWolosky Rebased and ready to go.

.NET Core implementation of `Assembly.Load` creates a new [AssemblyLoadContext](https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/assemblyloadcontext.md), isolating assemblies and ultimately resulting in dependency load related failures (e.g. microsoft#177).

This change loads Azure Storage assemblies into the default AppDomain or AssemblyLoadContext via `Assembly.LoadFrom`.
@riverar riverar force-pushed the coreclr-alc-support branch from f775a48 to 0270635 Compare August 24, 2020 01:51
@riverar
Copy link
Contributor Author

riverar commented Aug 24, 2020

(Edited commit msg to align with project's current state.)

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for providing this fix!

@HowardWolosky HowardWolosky merged commit bb0b4a0 into microsoft:master Aug 24, 2020
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 this pull request may close these issues.

2 participants