Repository navigation
Fix stack overflow exception when loading assemblies - #814
Conversation
|
George Fleming (@palladia) Can you test that this didn't break trying to load PSReadline on Linux? It shouldn't but this code has a lot of side-effects. |
|
It loaded PSReadLine without any problem. |
|
Andy Jordan (@andschwa) I don't understand why "Assembly.Load" was added to "Load" and "LoadFrom" in the first place. AssemblyLoadContext.Load is the fallback of Assemly.Load -- CoreCLR runtime first try to resolve the assembly from TPA list, if that fails, it calls AssemblyLoadContext.Load where we take over the resolution. Calling "Assembly.Load" within ALC.Load is bound to cause infinite loop ... |
ba32641 to
a370c13
Compare
|
Andy Jordan (@andschwa) And one minor comment on your change: it's intentional to avoid foreach loop in PowerShellAssemblyLoadContext.cs. Foreach loop will generate new enumerator objects, which cause extra overhead to be garbage collected. It's assumed that the code in this file will be called frequently, and thus avoiding foreach loop is a minor optimization. |
b38742f to
1bc2a9e
Compare
When an assembly that exists outside of the trusted platform assembly list but in the GAC on Windows was loaded, the use of `Assembly.Load` before the `LoadFromAssemblyPath` caused a recursive loop to occur. This happens because an `Assembly.Load` on a not-yet-loaded assembly is overridden, thus calling back into the `AssemblyLoadContext.Load` override. By attempting to load from the file path first, we avoid this loop. However, loading TPA assemblies by their path throws an exception, so now we catch this particular exception and attempt the load through `Assembly.Load`. This is safe for TPA assemblies since they're already loaded, thus the `Assembly.Load` does not re-load the assembly, but simply returns it.
|
Dongbo Wang (@daxian-dbw) You're right in that this is totally wonky. However, I'm going to merge it for now, with the intent of integrating your correct fix when it is available. |
I forgot to revert this in #814, thanks @daxian-dbw for the pointer.
I forgot to revert this in #814, thanks @daxian-dbw for the pointer.
Fix stack overflow exception when loading assemblies
I forgot to revert this in PowerShell#814, thanks @daxian-dbw for the pointer.
When an assembly that exists outside of the trusted platform assembly
list but in the GAC on Windows was loaded, the use of
Assembly.Loadbefore the
LoadFromAssemblyPathcaused a recursive loop to occur.By attempting to load from the file path first, we avoid this loop.
However, loading TPA assemblies by their path throws an exception, so
now we catch this particular exception and attempt the load through
Assembly.Load.Dongbo Wang (@daxian-dbw), I really don't like this solution, as it relies on catching the exception from the failed load-by-path. However, we do need to support the .NET CLI scenario with the PowerShell assemblies exist in the TPA. To do this properly I think means breaking the coupling of the type analysis and assembly loading, so that we can analyze SMA etc. for their cmdlets, without having to re-load them.
This change is