fix: If GetCurrentAppDomainId fails try GetDefaultDomain first (.netframework remote start issue)

mitchcapper commented 1 year ago

Thanks for a fantastic library! This does 3 things, if you want it split into 3 commits let me know:

Adds the cpp platform to the solution/project as it is what is generally used anyway
Checks the result of GetCurrentAppDomain() call, for whatever reason the result was ignored before, maybe to default to 0 incase of failure, but this can lead to startup aborts later when the appdomain is incorrect (adds a few more error states as well)
Fix an issue with loading in a .net framework application via a new remote thread. I found GetCurrentAppDomain would fail and couldn't find a good way to fix this. The other calls all succeeded. I did find that if I called GetDefaultDomain on the old runtime interface this would fix it so the call would then succeed. Technically if we import the AppDomain type we could pull the id right off of the result from GetDefaultDomain but not sure its needed. This should only change the hotpath for previous where the code would gloriously fail before (well assuming the appdomain was not 0, but the more commonly 1 for .net framework apps anyway).

mitchcapper commented 11 months ago

So sorry forgot to do as requested and follow up on this!

AaronRobinsonMSFT commented 11 months ago

So sorry forgot to do as requested and follow up on this!

No worries. We all get busy with the constant in/out of PRs and requested changes. I didn't want to step on your effort so I didn't do it myself. However, I like this fix and want to get it in this week :-)

AaronRobinsonMSFT commented 10 months ago

Thanks @mitchcapper !

AaronRobinsonMSFT commented 10 months ago

@mitchcapper A new package with these changes has been release - https://www.nuget.org/packages/DNNE/2.0.6

AaronRobinsonMSFT / DNNE

fix: If GetCurrentAppDomainId fails try GetDefaultDomain first (.netframework remote start issue) #178