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

Update host logic to support optional VM assembly #47481

Open
marek-safar opened this issue Jan 26, 2021 · 28 comments
Open

Update host logic to support optional VM assembly #47481

marek-safar opened this issue Jan 26, 2021 · 28 comments
Milestone

Comments

@marek-safar
Copy link
Contributor

For desktop scenarios on architectures where CoreCLR is not supported we need to use MonoVM as supported runtime. Today the host hardcodes libcoreclr name as the only one to look for which makes this problematic because MonoVM capabilities and internals are different.

This should unblock issues like #34202.

/cc @steveisok

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 26, 2021
@ghost
Copy link

ghost commented Jan 26, 2021

Tagging subscribers to this area: @vitek-karas, @agocke
See info in area-owners.md if you want to be subscribed.

Issue Details

For desktop scenarios on architectures where CoreCLR is not supported we need to use MonoVM as supported runtime. Today the host hardcodes libcoreclr name as the only one to look for which makes this problematic because MonoVM capabilities and internals are different.

This should unblock issues like #34202.

/cc @steveisok

Author: marek-safar
Assignees: -
Labels:

area-Host, untriaged

Milestone: -

@vitek-karas
Copy link
Member

Are we going to use the same ABI as coreclr.dll has, or is the plan to use different one for mono?
There's also the question of where this needs to "ship". All this logic lives in hostpolicy which ships next to coreclr.dll. My understanding is that all mono scenarios are self-contained (we don't ship shared frameworks with mono VM in it). That means there's no way for one hostpolicy dll to have to deal with both CoreCLR and MonoVM. So we could just build slightly different version of hostpolicy for the mono runtime packs - and that can use whatever ABI is needed (and also look for different name of the file).

It's is also possible to build the one hostpolicy which can handle both, but as far as I can tell there's no need for this (unless it helps us with packaging shipping - which might be the case).

@marek-safar
Copy link
Contributor Author

ABI would be the same as for coreclr and this is for shared framework scenarios where CoreCLR is not supported like s390x

@vitek-karas
Copy link
Member

I see - do we need to case where on one machine I have shared frameworks with both CoreCLR and MonoVM? (I hope not, otherwise framework resolution will become more complex than it already is).

@marek-safar
Copy link
Contributor Author

do we need to case where on one machine I have shared frameworks with both CoreCLR and MonoVM?

No, the simple case should be enough at least for now (although it would help with development/testing)

@vitek-karas
Copy link
Member

In that case we can build a different version of hostpolicy for these "mono" platforms.
/cc @VSadov - as this might affect the work to move hosting components from the installer subtree to some other place

@akoeplinger
Copy link
Member

I think we need some kind of runtime switch for choosing between mono/coreclr on the standard platforms too if we want to really unblock #34202 since right now we need to rename libmono to libcoreclr in the testhost for libraries testing.

@steveisok
Copy link
Member

It would be nice if we could have something like --vm mono as a runtime option and be able to switch between coreclr / mono in the same install. Assuming we have different names between libmono and libcoreclr, I think we would just have to put each runtime's System.Private.CoreLib in a separate directory. Not sure how easy that would be.

@vitek-karas
Copy link
Member

@steveisok this has been discussed in the past. It's definitely possible, but it's a rather large change as it would require changes to runtime packs, SDK, installers, ... There's also the question of breaking apps due to files lying in a different locations on disk (the typically typeof(object).Assembly.Location would now point to a subdirectory) and so on.

If we were to support installing both CoreCLR and MonoVM shared frameworks on the same machine, then the best solution would be to include some differentiator elsewhere. The problem is that currently the system doesn't have a place to do this - adding another level of directories will probably break too many things if we do this. And it still impacts pretty all parts of the toolchain.

If this would be only for development/test purposed, then I think the easiest way would be to simply create a new installation in a different location and rely on DOTNET_ROOT. This is already done in many places to handle different versions and such, and so it has broad support across our tooling. The downside is size on disk - it would have to be a full install of at least the runtime (so all libraries + runtime).

@akoeplinger
Copy link
Member

akoeplinger commented Jan 27, 2021

If this would be only for development/test purposed, then I think the easiest way would be to simply create a new installation in a different location and rely on DOTNET_ROOT

We'd still need some switch to tell the host to look for libmono instead. Or we could probe for libcoreclr and fallback to libmono if it doesn't exist? For architectures like s390x the libcoreclr check could be ifdefed out.

@vitek-karas
Copy link
Member

I think it's perfectly OK to accept more than one name for the runtime itself - so the fallback logic could definitely work. There will be another small gotcha - currently the host requires coreclr.dll to be listed in .deps.json - so to produce a valid MonoVM based framework it would now have to use the mono.dll in the .deps.json instead. It's definitely the cleaner way.

Just so that this is actually actionable - some clarifications:

  • What is the name of the mono runtime library we want the host to recognize? mono (plus all the required platform prefixes and suffixes).
  • Do we need any changes to the ABI through which hostpolicy communicates with the runtime library? (coreclr_initialize and so on)

@jkotas

This comment has been minimized.

@akoeplinger
Copy link
Member

so to produce a valid MonoVM based framework it would now have to use the mono.dll in the .deps.json instead. It's definitely the cleaner way.

As far as I know the .deps.json is generated based on the contents of the runtime pack so it should automatically get the libmono instead.

What is the name of the mono runtime library we want the host to recognize? mono (plus all the required platform prefixes and suffixes).

Yes.

Do we need any changes to the ABI through which hostpolicy communicates with the runtime library? (coreclr_initialize and so on)

I don't think so, we implement these right now:

MONO_API int STDAPICALLTYPE coreclr_initialize (const char* exePath, const char* appDomainFriendlyName,
int propertyCount, const char** propertyKeys, const char** propertyValues,
void** hostHandle, unsigned int* domainId);
MONO_API int STDAPICALLTYPE coreclr_execute_assembly (void* hostHandle, unsigned int domainId,
int argc, const char** argv,
const char* managedAssemblyPath, unsigned int* exitCode);
MONO_API int STDAPICALLTYPE coreclr_shutdown_2 (void* hostHandle, unsigned int domainId, int* latchedExitCode);
MONO_API int STDAPICALLTYPE coreclr_create_delegate (void* hostHandle, unsigned int domainId,
const char* entryPointAssemblyName, const char* entryPointTypeName, const char* entryPointMethodName,
void** delegate);

@vitek-karas
Copy link
Member

Re .deps.json:
The one in self-contained apps is generated from the runtime pack. But large part of this discussion is to support framework-dependent apps on mono VM. So the question is how are we going to produce that:

  • Microsoft.NETCore.App (currently built by the installer subset in dotnet/runtime would need to get a "mono" variant
  • It's probably easy enough for platforms which don't support CoreCLR - there we simply include MonoVM instead and ship it
  • How will this work for platforms which do support CoreCLR? This is a similar/related question to how are we differentiating the runtime packs between CoreCLR/MonoVM for RIDs which support both.

@akoeplinger
Copy link
Member

.deps.json for framework-dependent apps doesn't contain a reference to libcoreclr so that should not need any changes.

We already build variants of the runtime pack for platforms also supported by CoreCLR, there's an additional .Mono in the package name: https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=dotnet6&package=Microsoft.NETCore.App.Runtime.Mono.linux-x64&protocolType=NuGet&version=6.0.0-preview.2.21078.3

Consuming this is also already supported by the SDK since dotnet/sdk#11824

@vitek-karas
Copy link
Member

The problem is with the framework itself - it also contains .deps.json (specifically Microsoft.NETCore.App.deps.json) and that contains libcoreclr.so - you can also see this in the runtime pack which has this file it as well (and I think it will need to be modified for self-contained apps to use libmono instead.

The runtime packs solve the self-contained apps, for framework-dependent apps we would need to also produce at least .tar.gzip/.zip files (for installation to a private location and then using the DOTNET_ROOT as mentioned above).

@akoeplinger
Copy link
Member

Not sure I get what the problem with the .deps.json is, a mono-based shared framework like the one I linked lists libcoreclr.so but only because we renamed the file. If we rename it back to libmono.so then the .deps.json will be correct too.

@vitek-karas
Copy link
Member

Good - in that case it should work by just teaching hostpolicy about the different name.

@vitek-karas vitek-karas added this to the 6.0.0 milestone Jul 9, 2021
@vitek-karas vitek-karas removed the untriaged New issue has not been triaged by the area owner label Jul 9, 2021
@vitek-karas
Copy link
Member

If #34202 should be done in 6, then this needs to be done as well. Otherwise we should move both to future release.

@akoeplinger
Copy link
Member

I do think this should be done for 6, we just need to agree on the approach. I think given the timeframe I prefer probing for libcoreclr and falling back to libmono if it doesn't exist, that should be a very small change to the host.

@jkotas
Copy link
Member

jkotas commented Jul 9, 2021

@akoeplinger Why do we need to do it for 6 without #34202 ? It would be future proofing that rarely works in practice.

@akoeplinger
Copy link
Member

akoeplinger commented Jul 9, 2021

@jkotas my understanding is we should get #34202 done in 6.0 too, but it depends on this issue (i.e. we can't rename the library until the host can load libmono).

@jkotas
Copy link
Member

jkotas commented Jul 9, 2021

Ah ok, I got confused between #34202 and #49661 (should one be resolve as duplicate of the other?). #49661 was moved to .NET 7.

@akoeplinger
Copy link
Member

Yes, I closed #49661 as a duplicate.

@agocke agocke modified the milestones: 6.0.0, 7.0.0 Aug 9, 2021
@agocke agocke added this to AppModel Jul 13, 2022
@agocke
Copy link
Member

agocke commented Jul 25, 2022

Is this still an issue? I thought Mono was currently successfully using the host in non-mobile scenarios.

@steveisok
Copy link
Member

steveisok commented Jul 25, 2022

This is still an issue. We want to have the host recognize a vm lib other than libcoreclr.

@agocke
Copy link
Member

agocke commented Jul 25, 2022

I'll add it to 8.0, since this will likely not make it for 7.0.

@agocke agocke modified the milestones: 7.0.0, 8.0.0 Jul 25, 2022
@agocke agocke modified the milestones: 8.0.0, Future Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

6 participants