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

GetAssemblyTypesWithDynamicLinqTypeAttribute fails to type load failures #702

Closed
philr opened this issue May 2, 2023 · 4 comments
Closed
Assignees
Labels

Comments

@philr
Copy link

philr commented May 2, 2023

For .NET Framework, AbstractDynamicLinqCustomTypeProvider.GetAssemblyTypesWithDynamicLinqTypeAttribute() calls Assembly.GetExportedTypes() and handles a resulting ReflectionTypeLoadException.

Assembly.GetExportedTypes() will actually throw FileNotFoundException if an assembly cannot be loaded.

ReflectionTypeLoadException is only thrown by Assembly.GetTypes().

As a result, if there's a failure loading a type caused by a missing assembly, none of the types marked with DynamicLinqTypeAttribute in the assembly will be returned.

@StefH StefH self-assigned this May 2, 2023
@StefH StefH added bug question and removed bug labels May 2, 2023
@StefH
Copy link
Collaborator

StefH commented May 2, 2023

@philr Did you encounter this issue in your application, or did you check the source code?

Because the logic in the code is that if an assembly cannot be loaded, the definedTypes will be empty and no types will be returned for that assembly, but the next assembly will be analyzed.

@philr
Copy link
Author

philr commented May 2, 2023

I encountered this in a .NET Framework 4.7.2 application. System.Linq.Dynamic.Core is being called from within a sandboxed application domain where the not all of the dependencies of an assembly are available. None of the types marked with DynamicLinqTypeAttribute in that assembly were being picked up.

Assembly.GetExportedTypes() is throwing FileNotFoundException, so definedTypes doesn't get reassigned for the assembly in question (the catch all case in the code - effectively just skipping the assembly).

The fix seems to be to use Assembly.GetTypes() instead. The ReflectionTypeLoadException handler will then be invoked if there are some types that cannot be loaded and it will check the types that can be loaded (reflectionTypeLoadException.Types.WhereNotNull on line 141).

I've created a custom provider as a workaround for now:

class CustomDynamicLinqCustomTypeProvider : DefaultDynamicLinqCustomTypeProvider
{
    private HashSet<Type> _customTypes;

    public CustomDynamicLinqCustomTypeProvider() : base(false)
    {
    }

    public override HashSet<Type> GetCustomTypes()
    {
        if (_customTypes == null)
        {
            var customTypes = base.GetCustomTypes();
            Type[] types;

            try
            {
                types = typeof(TypeFromMyAssembly).Assembly.GetTypes();
            }
            catch (ReflectionTypeLoadException e)
            {
                types = e.Types;
            }

            foreach (var type in types.Where(t => t != null && t.IsPublic && t.GetCustomAttribute<DynamicLinqTypeAttribute>() != null))
            {
                customTypes.Add(type);
            }

            _customTypes = customTypes;
        }

        return _customTypes;
    }
}


ParsingConfig.Default.CustomTypeProvider = new CustomDynamicLinqCustomTypeProvider();

@StefH
Copy link
Collaborator

StefH commented May 4, 2023

@philr
If you have time, can you verify that PR?

@philr
Copy link
Author

philr commented May 11, 2023

@StefH I've added a review to the PR with a few comments.

@StefH StefH closed this as completed May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants