Add LoadAssemblyFromNativeMemory function to load assemblies in memory from a native PowerShell host
#14652
Conversation
|
|
|
@SteveL-MSFT @joeyaiello @iSazonov this is the only thing required to make PowerShell native hosts possible from any native language using nothing but the existing PowerShell files, as seen on twitter! Help me get this in an acceptable state and I promise you'll see a special Rust library published soon making it possible to load and host PowerShell 7 with zero link-time dependencies, and zero installation required other than PowerShell. If you have "pwsh", then you now have it usable inside your native host process as well. |
There was a problem hiding this comment.
/cc @PaulHigin @TravisEz13 Please review a security of the new public API.
| /// <summary> | ||
| /// Provides helper functions to faciliate calling managed code from a native PowerShell host. | ||
| /// </summary> | ||
| public static class NativeHost |
There was a problem hiding this comment.
My post for discussion from original issue: PowerShellAssemblyLoadContext is internal :-( Utils.cs too. I suggest to start the prototype with public sealed class UnmanagedPSEntry. Alternative is to put the public class in SMA.
|
Security Working group reviewed, and we don't find a security issue in the public API. |
|
@TravisEz13 thanks! Anybody knows what is causing some checks to fail? It appears unrelated to the changes in this PR: |
I restarted CI-Windows. |
|
@anmenaga @daxian-dbw I see you've been assigned as reviewers, what would be the next step? It's a small change, but there are things that are open to debate, such as:
I'm not very familiar with ALCs, but loading my bindings to the PowerShell class inside the default ALC works just fine for me. If someone can foresee a potential limitation with ALCs and knows more about them than I do, it would be work pointing it out now. |
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
I am waiting on one of the reviewers to tell me what the next step is :) |
|
Somehow we need to show that the failures are not related to your change. |
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
| /// <summary> | ||
| /// Provides helper functions to faciliate calling managed code from a native PowerShell host. | ||
| /// </summary> | ||
| public static class NativeHost |
There was a problem hiding this comment.
For this to be a public API, I think a more specific name (and one we're likely not to want to use for a different purpose) would be ideal.
But also it feels like this should probably be another method on either UnmanagedPSEntry or ManagedPSEntry, since those classes exist for this exact purpose
There was a problem hiding this comment.
do you want to rename "NativeHost" to "UnmanagedHost", "UnmanagedPSHost" or "UnmanagedPSHostHelper"? name your pick, I'll update the PR accordingly. If you feel like this should be moved to UnmanagedPSEntry I can put it in there, it just felt a bit out of place since there was pretty much a single entry point function in that class.
However, I'll go with whatever you think feels right, I don't really care how it is named and where it is placed in the end, as long as it is included somewhere :)
There was a problem hiding this comment.
Ideally I'd like to find consensus with @iSazonov and @daxian-dbw on what the best course of action is. I think my own preference is to put it on UnmanagedPSEntry, but want to discuss it briefly first just so we're comfortable with the API.
Also, I'm not sure what the overhead on testing this would be, but without one I imagine there's an increased likelihood of regression
There was a problem hiding this comment.
@rjmholt it may be a difficult one to test automatically, because it's meant to be called from an unmanaged host. However, we could make it callable from managed code, and maybe use a test similar to whatever is currently used to load assemblies in memory?
There was a problem hiding this comment.
Ideally I'd like to find consensus with @iSazonov and @daxian-dbw on what the best course of action is. I think my own preference is to put it on
UnmanagedPSEntry, but want to discuss it briefly first just so we're comfortable with the API.
Perhaps UnmanagedPSEntry is too specific that coming from Windows PowerShell. Do we really need always ConsoleHost dll in custom projects? If no it is better to place the new API in SMA.
|
@SteveL-MSFT @rjmholt @iSazonov @daxian-dbw @PaulHigin @TravisEz13 @anmenaga I would like to get this PR unstuck from the mud. It's unclear from the previous discussion what change would make it finally clean enough for a merge. There's a lot of "this might be better, maybe we should move it here" but nothing firm from one of the 4 assigned reviewers yet? I really do not care what the class name and function name is, I just want to know what needs to be changed to make it good for a merge, especially before 7.2 becomes final? I would hate to have to wait for 7.3 just for 5 lines of code, but I would be fine with requiring 7.2 as a baseline just for this. Please tell me exactly what to fix, I'll do it once, and then we can move on. So, I'm asking, what changes need to be done to make the current PR acceptable, please? :) |
|
I reread all related links to refresh my memory. The main fact is that .Net team has no plans now and near future to implement an solution for supporting native host scenarios and recommends to use custom wrap managed dll which can be called from native code. Based on the fact I see two way we have.
/cc @SteveL-MSFT @JamesWTruher @daxian-dbw who could weight and select a direction we will follow.
|
|
@iSazonov thank you for the detailed follow up. I noticed that you pointed to my larger set of bindings, and I just want to clarify that my PR is to add a single function that makes it possible to load any set of bindings not distributed with PowerShell. In other words, a single native function export to load your own bindings, and not the whole set of bindings, making it a "bring your own bindings" type of solution, but no additional file or installation required to load the bindings. Your first suggestion is still what I'd like: just include this single function somewhere into PowerShell and we can take it from there without further modifications to PowerShell. My very first use case for this is a Rust crate that will enable loading and using PowerShell entirely at runtime without redistributing PowerShell, and without link time dependencies. This is what I could validate with my proof-of-concept in C, and since then I've been working very hard to find a solution to the last problem of requiring at last one additional file on disk distributed separately. The second suggestion of making a wrapper DLL as a shared solution would unfortunately greatly increase the scope of the project and take it in a completely new direction. Because it's very hard to know how foreign language XYZ would deal with the bindings, I wouldn't even consider trying to do it that way. This being said, nothing prevents implementing the first suggestion (single native function export + bring your own bindings) and then working on a useful set of bindings that most people would be fine with. However, that would be a totally different story and out-of-scope for the current work. Since the security team did not find any objections to the current PR, maybe it's already good enough for a possible merge, unless someone wants to request modifications? At this point any change request would likely be non-functional in nature (name and location in the code), but there seems to be no objection on the actual code change made here. So, who can give their seal of approval on this one? @SteveL-MSFT ? :) |
I have reviewed the existing API and believe that it has nothing to do with the new API. The existing API comes from Windows PowerShell and .Net Framework - it can not be used with .Net Core and we could remove it at all. (Or is it used on Windows Nano?) So I think we should put the new API in SMA dll as new standalone static class (with a comment about limited support as I mentioned above). |
|
@iSazonov thank you, the new API has nothing to do with the old one, so I do agree it has to be in its own standalone static class (which is currently the case). I'm no expert in PowerShell internals, can you tell me what SMA stands for and where it is? In this case, all I'd need is to move the class (currently named NativeHost in NativeHost.cs) to the SMA DLL, if you could just point me to where that is :) |
That's even better, my primary concern was ABI stability just because of the return type, if you make it an int with a known success/failure return value (without necessarily defining error codes and an enum) that would be absolutely perfect! |
|
I believe we still need to declare an experimental feature name in this list, even though we won't really use it for anything. @SteveL-MSFT, any suggestion on the feature name? |
|
Security working group:
Once we have code for the logging, the security working group should review the PR This can be a separate PR |
|
I chose the name |
|
A new test has been failing consistently in mac CIs for this PR: Further investigation is needed. /cc @SteveL-MSFT |
…rShell inside native processes 100% at runtime
…UnsafeAssemblyLoad as unsafe
4b93ba8 to
97a30b1
Compare
|
The name of the ExperimentalFeature seems fine. I checked out this branch and it doesn't have that test case that's failing which is quite strange as CI is running it? I rebased to pick up the code changes related to that test change. |
|
@awakecoding Congratulations! 😄 |
Woohoo! Now we can start looking for the next steps, I think there were mentions of many new tickets related to this one? |
…emory in a native PowerShell host (PowerShell#14652)
|
🎉 Handy links: |
PR Summary
Add a new NativeHost class in Microsoft.PowerShell.ConsoleHost.dll with a LoadAssemblyData function callable from unmanaged code to load assemblies in memory. This fixes #14641 and dotnet/runtime#46652 for PowerShell. This modification has been confirmed to work in a proof-of-concept PowerShell native host.
PR Context
Long story short, the ultimate goal is to make it possible for any native language such as C/C++/Rust to find, load, and host PowerShell 7 entirely at runtime, using nothing but the existing PowerShell installation. This is possible by loading the hostfxr library found alongside PowerShell dynamically to then load the PowerShell runtime config. The last step is to load bindings to make the bridge between unmanaged to managed code possible, and wrap functions from Microsoft.PowerShell.SDK.
Loading the precompiled bindings is currently only possible from a file with the hostfxr APIs, which is why I originally approached the .NET runtime team first (dotnet/runtime#46652) but eventually exhausted all options. I then opened an issue with PowerShell (#14641) to see if this could be fixed outside of the .NET runtime, after all, we only need one helper function exported by one of the PowerShell assemblies.
PR Checklist
This is my first contribution to the core PowerShell project, and I perfectly understand that questions are going to be asked: why is this necessary, why a new class, why this function, etc. Please just work with me to find the best place to put this single helper function meant to facilitate loading PowerShell inside a native host. I deliberately used just one function and didn't try exporting more functions because it is a small change that will require no further changes inside PowerShell. I placed it inside its own class inside Microsoft.PowerShell.ConsoleHost.dll, but I'm open to suggestions.
In other words: if I can load my assembly in memory, then I can load all the bindings I need for the native language of my choosing. It also becomes a lot easier to choose exactly how those bindings will handle marshaling, and which functions they expose to unmanaged callers. This is a one-time thing that unlocks extensibility forever because you can load anything into the assembly context to fit your needs.
The helper function always loads the assembly into the default AssemblyLoadContext. My thinking is that if I ever need to deal with non-default ALCs, I can still add more functions callable from native code to deal with non-default ALCs, even if those helper functions are first loaded in the default ALCs. However, if there are easier ways to extend the NativeHost class, I'm open to suggestions, but I'd rather keep changes as small as possible to increase the chances of this PR being accepted.