Runtime: Extend AssemblyLoadContext API for diagnostics

Created on 10 Jan 2019  路  14Comments  路  Source: dotnet/runtime

To simplify diagnostics for System.Runtime.Loader.AssemblyLoadContext, add the following APIs.

```C#
// Allow naming each AssemblyLoadContext
protected AssemblyLoadContext (String name);
public String Name { get; }

// Override the ToString to return Name or type name
public override string ToString();

// Set of ALCs
public static IEnumerator Contexts { get; }

// Set of Assemblies in the ALC
public IEnumerable GetAssemblies();
```

The name will make it more convenient to understand what is happening in the debugger. It will also make any tracing easier to understand.

  • The Default ALC would be constructed with name = "Default".
  • The Assembly.LoadFile(path) generated ALCs would be constructed with name = "Assembly.LoadFile()" .
  • Custom AssemblyLoadContext implementations could provide a logical name. For legacy usage, these would default to the Custom ALC type name.

The set of assemblies and set of ALCs are convenience APIs that help make understanding isolation design flaws simpler. They provide a convenient mechanism to iterate through ALCs and their contents.

api-needs-work area-System.Runtime

Most helpful comment

anonymous AssemblyLoadContext created by Assembly.LoadFile(path)

Maybe Assembly.LoadFile(<path>) ? The friendly name should allow people to quickly guess where the context came from. Including the API name that created the context verbatim should help.

All 14 comments

/cc @vitek-karas @swaroop-sridhar @jeffschwMSFT @sbomer @jkotas

IEnumerator<WeakReference<AssemblyLoadContext>> Contexts

Why does this return a WeakReference? If the caller wants to store it for longer, they can create weak reference themselves.

I guess I knew internally it would need to be stored as weakreferences to prevent unloadability issues. I guess you are right. The API does not need to use weak references. I'll update it.

Looks good to me

What do we expect the diagnostic info to be? A short name; or a long (e.g. multi line) string? If we expect it to be a short name - as suggested by Default for the default context, we may want to call it Name or FriendlyName; and have property that returns it. We have prior art for this: AppDomain.FriendlyName, Thread.Name.

Originally @vitek-karas was proposing it to be a friendly name. I was concerned about the anonymous AssemblyLoadContext created by Assembly.LoadFile(path). I guess the name in this case could be
Anonymous-Load.File(<path>) (or Anonymous-Load.File(<path>)-<Sequence> if we want a more unique name).

anonymous AssemblyLoadContext created by Assembly.LoadFile(path)

Maybe Assembly.LoadFile(<path>) ? The friendly name should allow people to quickly guess where the context came from. Including the API name that created the context verbatim should help.

I vote for name (should discuss in API review if we should also add a Name property or not).
I would not include detailed info in this (as in callstacks and such), The intent is for this to show up in the debugger (ToString will do that) and thus it should be reasonably short to be useful.

I think we should describe some sample scenarios for each of the additions.

  • The name and ToString are discussed above - help identify the context in the debugger. Can also be used in tracing later on.
  • Assemblies - ??? - this one I can see some usefulness (during debugging)
  • Contexts - ??? - this one is a bit confusing to me...
  • Assemblies - ??? - this one I can see some usefulness (during debugging)
  • Contexts - ??? - this one is a bit confusing to me...

If I wanted a visual representation of the isolation boundaries, I would want the set of ALC's and the set of Assemblies in each ALC.

Neither one of these is strictly necessary as they can currently be derived from iterating through the assemblies in the default AppDomain.GetAssemblies() and calling AssemblyLoadContext.GetLoadContext(). I am leaning to adding them because I would argue they belong in a functionally complete interface.

To be consistent with AppDomain, we may want to change the proposal to

public System.Reflection.Assembly[] GetAssemblies();

I am not sure if an equivalent interface for unmanaged assemblies would be useful.

I updated the proposal to at least partially address @vitek-karas comment and I changed the Assemblies property to GetAssemblies() to match AppDomain

To be consistent with AppDomain, we may want to change the proposal to public System.Reflection.Assembly[] GetAssemblies()

If GetAssemblies() returns array, GetLoadContexts() should return array too.

Wrt array vs. IEnumerable<T>: The original .NET Framework APIs tend to return arrays for enumerations because of we did not have generics. We tend to prefer to return IEnumerable<T> from new APIs.

@jkotas done

The .ctor should be public - especially once we make the class non-abstract, then it should be possible to instantiate the class without deriving from it.

I don't see the value in trying to make some of the APIs look the same as they did on AppDomain. ALC is very different from AppDomain so there's probably little value in trying to make it feel familiar. I would rather vote for consistency. So either both Contexts and Assemblies as properties or both as Get... methods. Not sure what the API guidelines are (if any) about get-only properties.

Otherwise looks good.

Moved to dotnet/corefx#34791. @vitek-karas comment was addressed in the move.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aggieben picture aggieben  路  3Comments

Timovzl picture Timovzl  路  3Comments

matty-hall picture matty-hall  路  3Comments

nalywa picture nalywa  路  3Comments

chunseoklee picture chunseoklee  路  3Comments