Expand host_runtime_contract to get assembly names and resolve their paths #100503

steveisok · 2024-04-01T23:50:12Z

This change continues down the path of making the host_runtime_contract structure the way the runtime can query information about the host.

get_assemblies: function that gets the list of assemblies (name+extension) known by the host.
destroy_assemblies: function that cleans up the what was returned by get_assemblies when you're done with it
resolve_assembly_to_path: function that when given an assembly name (no extension) will return the full path known by the host.
entry_assembly: string containing the full path to the designated entry assembly

Contributes to Allow modification of entry assembly and probing after startup hooks are run #99883

…g path fields. Modified the host to read the values into these structures

dotnet-policy-service · 2024-04-01T23:50:35Z

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

steveisok · 2024-04-02T00:00:52Z

@elinor-fung a couple of items I'm wondering about:

Should the strings be utf-16 instead of utf-8 starting in the host? I see one of the roles of HostInformation is to transform strings, but I'm starting to wonder if utf-16 makes more sense because the runtime can just use the contract.
What are the rules around memory between the host and the runtime?

jkotas · 2024-04-02T01:31:57Z

src/native/corehost/host_runtime_contract.h

+	char** dirs;
+};
+
+struct probing_path_properties


Should this rather be a callback?

A performance flaw of the current system is that the host has to push paths to the runtime upfront. It introduces a lot of unnecessary copying and parsing. If we are redoing how this works, we should address this performance flaw. Pushing a list of full paths and base-names maybe a bit more efficient than the current system, but it still leaves a lot on the table.

My initial thought was no callback, since the runtime requires all the paths on startup and would immediately have to call it. However, avoiding the extra copying would definitely be better than avoiding the extra call back to the host and would answer the question of the memory between host and runtime (for the properties, it was intentionally all allocated by the runtime).

Runtime does not fundamentally require all paths on startup. All that the runtime does with the paths during startup is to store them in a hashtable with simple name as the key.

Maintaining this hashtable on the host side would open a number of opportunities for memory optimizations. For example, we do not need to materialize all full paths eagerly. Instead, we can remember a few common prefixes and materialize the full path from the common prefix and suffix on demand.

this hashtable

This hashtable needs to be case-insensitive (assembly binding is case-insensitive). Case-insensitive hashing and comparisons are tricky. The host does not seem to have the infrastructure for it.

So maybe the best design is to push the simple assembly names from the host eagerly, let the runtime maintain the hash table, but then use a callback to resolve the simple name into a full path. It would avoid potential mismatches in case-insensitive behavior for non-ascii file names, etc.

jkotas · 2024-04-02T01:33:28Z

src/native/corehost/host_runtime_contract.h

 struct host_runtime_contract
 {
    size_t size;

    // Context for the contract. Pass to functions taking a contract context.
    void* context;

+    char* entry_assembly;


Nit: Should any new things be added to the end of this structure so that it grows in backward compatible way?

jkotas · 2024-04-02T01:48:32Z

src/coreclr/hosts/corerun/corerun.cpp

                });

            // Add to the TPA.
            tpa_list << tmp;
        }
    }

+    tpa_items->assembly_count = static_cast<uint32_t>(name_path_map.size());


There is a number of apps out there that host coreclr.dll directly using TRUSTED_PLATFORM_ASSEMBLIES and other key/value pairs. We had the direct coreclr.dll hosting documented at learning.microsoft.com for a while. There are scenarios where hosting via host policy is too inflexible.

Are we going to break the existing coreclr.dll hosting model based on TRUSTED_PLATFORM_ASSEMBLIES and other key/value pairs or are we keeping it around?

If we are keeping it around, we may want to keep corerun intact so that we get some test coverage for it. corerun is our non-shipping test-only host and it is fine for it to leave some perf on table.

If we are going to break it, it will be a significant breaking change that will require a lot of work to push through.

Are we going to break the existing coreclr.dll hosting model based on TRUSTED_PLATFORM_ASSEMBLIES and other key/value pairs or are we keeping it around?

We do not plan to break it. The runtime should continue to respect the key/value pairs (but hostpolicy / default hosting won't use it) and getting the properties (AppContext or hostfxr) should continue to work (but on demand).

We do not plan to break it.

Ok, we should think about having some test coverage. corerun test host would be a good place for it - keeping it intact should be the easiest option.

jkotas · 2024-04-02T01:52:44Z

Contributes to #99883

As we have discussed before, it would be useful to agree on the design of the public APIs that we are going to introduce to address the scenario first.

If this change is going to fix some of the performance issues of the current hosting scheme, that sounds good. I am not sure how it helps with #99883.

jkotas · 2024-04-02T01:55:03Z

src/coreclr/hosts/corerun/corerun.cpp

                });

            // Add to the TPA.
            tpa_list << tmp;
        }
    }

+    tpa_items->assembly_count = static_cast<uint32_t>(name_path_map.size());
+    tpa_items->assembly_filepaths = new char*[tpa_items->assembly_count];


Similarly, there is a lot of apps and libraries that depend on the existing documented key-value pairs in appcontext:

https://learn.microsoft.com/en-us/dotnet/core/dependency-loading/default-probing

https://stackoverflow.com/questions/75314798/is-there-a-net-framework-equivalent-to-cores-appcontext-getdatatrusted-platf

Are we going to break these or are these key/value pairs going to continue to work?

The plan is not to break anything. I gather from @elinor-fung that the host_runtime_contract usage is intended to be gradual.

vitek-karas · 2024-04-02T11:02:29Z

src/native/corehost/host_runtime_contract.h

+	char** dirs;
+};
+
+struct probing_path_properties


Naming nit: I would not use "probing". One of the goals of the .NET Core redesign of hosting was to avoid spurious probing for files on disk. The current implementation is a bit half-way-there, but realistically that's more of a bug than feature. TPA is already on the no-probing plan. And native and resource files are actually also no-probing inside hostpolicy, we just condense that information into the list of paths when calling into CoreCLR. Now I don't know if this is the time to change it (it can be a breaking change for some apps), but at least in terms of naming I would like to be clear that "We don't want to probe in the runtime" (note that hostpolicy does some amount of probing, but it does it so that the runtime doesn't have to).

Thanks! I anticipated having to rename things :-).

I would like to add here that we have an existing structure called probe_paths_t. We have to make sure that whatever we end up naming our probing paths stuff is clear enough to be differentiated from probe_paths_t. To not make the code more complicated to understand :)

jkotas · 2024-04-02T16:47:59Z

src/native/corehost/host_runtime_contract.h

+
+struct probing_path_properties
+{
+    trusted_platform_assemblies trusted_platform_assemblies;


Another naming nit: If we are redoing how this works, we should drop "trusted" and "platform" from the names of new things.

This name is left-over from Silverlight. There is nothing trusted or platform about assemblies in this list. If we go with the callback, a good name can be resolve_assembly_to_path - the callback would be native equivalent of existing managed API https://learn.microsoft.com/dotnet/api/system.runtime.loader.assemblydependencyresolver.resolveassemblytopath .

fanyang-mono · 2024-04-03T14:35:57Z

src/native/corehost/hostpolicy/deps_resolver.cpp

-        output->append(item.second.resolved_path);
-        output->push_back(PATH_SEPARATOR);
-    }
+    // Now that we have the complete list, store it in the host_runtime_contract and as a ; delimted string.


We want to store TPA list as an array instead of a string with delimiter.

From my understanding, we want to either stick to the string or use both to keep backwards compatibility.

Assuming I understand the feedback correctly, both in the runtime. What we'll angle to do is make the host by default use the callback method as opposed to passing in TRUSTED_PLATFORM_ASSEMBLIES

In the new model, the long TRUSTED_PLATFORM_ASSEMBLIES string with all paths concatenated should be only created on demand when the user code asks for it; and we should make sure that it does not happen in any of our mainline scenarios.

…m assembly names and the ability to resolve their paths

… list

dotnet-policy-service · 2024-05-17T02:45:53Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

…-contract

dotnet-policy-service · 2024-07-17T16:18:04Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

steveisok added 4 commits March 21, 2024 12:28

Messing around with extending host_runtime_contract

e957f4d

Update the host_runtime_contract to contain entry assembly and probin…

bb99691

…g path fields. Modified the host to read the values into these structures

Make sure to use char_t in the contract

8dd3eac

Switch host_runtime_contract strings back to char*

580dcb8

steveisok requested a review from elinor-fung April 1, 2024 23:50

dotnet-issue-labeler bot added the area-Host label Apr 1, 2024

steveisok requested review from fanyang-mono and ivdiazsa April 1, 2024 23:50

dotnet-policy-service bot assigned steveisok Apr 1, 2024

jkotas reviewed Apr 2, 2024

View reviewed changes

This was referenced Apr 2, 2024

Segmentation fault in System.Text.RegularExpressions.Tests #93206

Closed

System.Net.Http.Functional.Tests.HttpMetricsTest_Http11_Async_HttpMessageInvoker Test failure #96407

Closed

vitek-karas reviewed Apr 2, 2024

View reviewed changes

jkotas reviewed Apr 2, 2024

View reviewed changes

fanyang-mono reviewed Apr 3, 2024

View reviewed changes

steveisok added 2 commits April 11, 2024 14:34

Provide a way for the runtime to ask the host for the list of platfor…

4879bf5

…m assembly names and the ability to resolve their paths

Clean ups

93ab723

Add a destory callback on the host side to clean up the host assembly…

18d2488

… list

dotnet-policy-service bot closed this May 17, 2024

steveisok reopened this May 17, 2024

steveisok added 7 commits May 22, 2024 11:28

Fixed some usage shennanigans for the osx / linux coreclr build

0a0b40d

Adjust to get mono to build

7d8b671

Merge remote-tracking branch 'upstream/main' into expand-host-runtime…

41ede32

…-contract

Update GetProperty usage

5ae1b01

Merge remote-tracking branch 'upstream/main' into expand-host-runtime…

fef9e54

…-contract

Merge remote-tracking branch 'upstream/main' into expand-host-runtime…

4e679b4

…-contract

Merge remote-tracking branch 'upstream/main' into expand-host-runtime…

ee89ec3

…-contract

steveisok changed the title ~~Expand host_runtime_contract to include the entry assembly and probing paths~~ Expand host_runtime_contract to get assembly names and resolve their paths Jun 12, 2024

steveisok added 2 commits June 17, 2024 10:34

Start to modify tests around modified contract

9fd6e6c

Merge remote-tracking branch 'upstream/main' into expand-host-runtime…

28bcc7c

…-contract

dotnet-policy-service bot closed this Jul 17, 2024

github-actions bot locked and limited conversation to collaborators Aug 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand host_runtime_contract to get assembly names and resolve their paths #100503

Expand host_runtime_contract to get assembly names and resolve their paths #100503

steveisok commented Apr 1, 2024 •

edited

Loading

dotnet-policy-service bot commented Apr 1, 2024

steveisok commented Apr 2, 2024

jkotas Apr 2, 2024

elinor-fung Apr 2, 2024

jkotas Apr 2, 2024 •

edited

Loading

jkotas Apr 2, 2024

jkotas Apr 2, 2024

jkotas Apr 2, 2024

elinor-fung Apr 2, 2024 •

edited

Loading

jkotas Apr 2, 2024

jkotas commented Apr 2, 2024

jkotas Apr 2, 2024

steveisok Apr 2, 2024

vitek-karas Apr 2, 2024

steveisok Apr 2, 2024

ivdiazsa Apr 3, 2024

jkotas Apr 2, 2024

fanyang-mono Apr 3, 2024

ivdiazsa Apr 3, 2024

steveisok Apr 3, 2024

jkotas Apr 3, 2024 •

edited

Loading

dotnet-policy-service bot commented May 17, 2024

dotnet-policy-service bot commented Jul 17, 2024

Expand host_runtime_contract to get assembly names and resolve their paths #100503

Expand host_runtime_contract to get assembly names and resolve their paths #100503

Conversation

steveisok commented Apr 1, 2024 • edited Loading

dotnet-policy-service bot commented Apr 1, 2024

steveisok commented Apr 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elinor-fung Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas commented Apr 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

dotnet-policy-service bot commented May 17, 2024

dotnet-policy-service bot commented Jul 17, 2024

steveisok commented Apr 1, 2024 •

edited

Loading

jkotas Apr 2, 2024 •

edited

Loading

elinor-fung Apr 2, 2024 •

edited

Loading

jkotas Apr 3, 2024 •

edited

Loading