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

Resolved references should align when loading a self contained folder #580

Open
ds5678 opened this issue Sep 2, 2024 · 2 comments
Open
Labels
bug dotnet Issues related to AsmResolver.DotNet
Milestone

Comments

@ds5678
Copy link
Contributor

ds5678 commented Sep 2, 2024

Problem

I want to load a self-contained directory of assemblies, and have all the references line up. This is extremely common for Unity games.

internal class Program
{
    static void Main(string[] args)
    {
        AssemblyResolver resolver = new AssemblyResolver(args[0]);

        RuntimeContext context = new(new DotNetRuntimeInfo(DotNetRuntimeInfo.NetFramework, new Version(4, 0, 0, 0)), resolver);

        List<AssemblyDefinition> assemblies = new();
        foreach (string path in Directory.EnumerateFiles(args[0], "*.dll"))
        {
            AssemblyDefinition assembly = AssemblyDefinition.FromFile(path, new ModuleReaderParameters(context));
            assemblies.Add(assembly);
        }

        //Using the assembly list
    }

    private sealed class AssemblyResolver : AssemblyResolverBase
    {
        public AssemblyResolver(string directory) : base(new ModuleReaderParameters(directory))
        {
            SearchDirectories.Add(directory);
        }

        protected override string? ProbeRuntimeDirectories(AssemblyDescriptor assembly)
        {
            return null;
        }
    }
}

I know mscorlib gets loaded twice because I have TypeDefinition objects that are signature equal (SignatureComparer), but not reference equal.

Ideal Solution

I propose we add AssemblyDefinition.FromFiles and ModuleDefinition.FromFiles.

Workaround

This is how I'm currently working around the issue.

private static List<ModuleDefinition> LoadModules(string directory)
{
	AssemblyResolver resolver = new AssemblyResolver(directory);

	return Directory.EnumerateFiles(directory, "*.dll")
		.Order()
		.Select(resolver.LoadModuleFromFile)
		.ToList();
}

private sealed class AssemblyResolver : AssemblyResolverBase
{
	private readonly ConcurrentDictionary<string, AssemblyDefinition> fileCache = new();
	public AssemblyResolver(string directory) : base(new ModuleReaderParameters(directory))
	{
		SearchDirectories.Add(directory);

		ReaderParameters.RuntimeContext = new RuntimeContext(new DotNetRuntimeInfo(DotNetRuntimeInfo.NetFramework, new Version(4, 0, 0, 0)), this);
	}

	protected override string? ProbeRuntimeDirectories(AssemblyDescriptor assembly)
	{
		throw new NotSupportedException();
	}

	public ModuleDefinition LoadModuleFromFile(string path)
	{
		return LoadAssemblyFromFile(path).ManifestModule!;
	}

	protected override AssemblyDefinition LoadAssemblyFromFile(string path)
	{
		if (fileCache.TryGetValue(path, out AssemblyDefinition? assembly))
		{
			return assembly;
		}
		else
		{
			assembly = base.LoadAssemblyFromFile(path);
			fileCache.TryAdd(path, assembly);
			return assembly;
		}
	}
}
@Washi1337 Washi1337 added bug dotnet Issues related to AsmResolver.DotNet labels Sep 4, 2024
@Washi1337 Washi1337 added this to the 6.0.0 milestone Sep 4, 2024
@Washi1337
Copy link
Owner

The problem here is that currently AssemblyDefinition::FromXXX is kind of independent of RuntimeContext and its assembly resolver. It always produces a new instance of AssemblyDefinition, regardless of whether this was already added to a context's resolver cache or not (eg via dependency resolution).

I currently see two solutions to this problem:

  1. We change the behavior of AssemblyDefinition::FromXXX to have it first check the cache of the RuntimeContext's assembly resolver, see if there's already an assembly with the same signature and if so return that one instead of a new instance. This results in 0 changes for end-users, but may be confusing if loading a second instance is actually required, thus costing in predictability of the API.
  2. We leave AssemblyDefinition::FromXXX as is, but instead add a third way of loading assemblies into the RuntimeContext by adding LoadFromXXX methods to RuntimeContext that do this check. This keeps things decoupled and arguably results in a more predictable API, at the cost of yet another API for loading files to be introduced.

Personally I am of the opinion that option 2 is best, unless there are other options I haven't explored yet.

@ds5678
Copy link
Contributor Author

ds5678 commented Oct 7, 2024

In regards to 1, if a runtime context is being provided, it would be intuitive for it to return an already loaded assembly instead of loading a duplicate into the runtime context.

In regards to 2, I would love a RuntimeContext::LoadFromFolder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dotnet Issues related to AsmResolver.DotNet
Projects
None yet
Development

No branches or pull requests

2 participants