toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.51k stars 260 forks source link

Use of Type.GetType in OverrideRuntimeNodeWriterTemplateTypeNamePhase causes crash in Azure functions #322

Closed antichaosdb closed 4 years ago

antichaosdb commented 4 years ago

We've been running Razorlight successfully in Azure functions (v2 runtime) for a few months. Late last week we started seeing this error: Could not load file or assembly 'Microsoft.AspNetCore.Razor.Language, Culture=neutral, PublicKeyToken=null'. Invalid pointer (Exception from HRESULT: 0x80004003 (E_POINTER))

Stack trace showed this caused by this line in OverrideRuntimeNodeWriterTemplateTypeNamePhase.Register

defaultRazorCSharpLoweringPhase = builder.Phases.SingleOrDefault(x => x.GetType() == Type.GetType("Microsoft.AspNetCore.Razor.Language.DefaultRazorCSharpLoweringPhase, Microsoft.AspNetCore.Razor.Language"));

Quite what they changed in Azure to cause this to start to throw I have no idea, but I was able to fix it by using less reflection:

defaultRazorCSharpLoweringPhase = builder.Phases.SingleOrDefault(x => x.GetType().Name == "DefaultRazorCSharpLoweringPhase");

Hope this helps someone else.

Our engine build is _engine = new RazorLightEngineBuilder() .UseMemoryCachingProvider() .UseEmbeddedResourcesProject(typeof(RazorEmailGenerator)) .SetOperatingAssembly(typeof(RazorEmailGenerator).Assembly) .ExcludeAssemblies("System.Threading.AccessControl", "System.Security.Permissions") .Build();

As has been noted elsewhere, also need this in Entry project file

true true
jzabroski commented 4 years ago

I've never used Azure Functions, so I don't quite understand this comment. It's a bit unfortunate since I keep seeing customers in my various projects mention Azure Functions, even using them in ways I wouldn't expect, but I just don't use it. I scanned the documentation today searching for information to query details about the Azure Functions environment, but I dont see anything.

Is there a way I can advise people to log info about their Azure Functions environment?

antichaosdb commented 4 years ago

The only meaningful variable is the runtime version, which can be viewed on the Function app Setting page. Ours is currently 2.0.12998.0. Azure rolls out updates to this without warning, and sometimes it breaks stuff, as it did for us a few days ago. Fortunately we got a good stack trace off Razorlight so we were able to see the error.

Dynamically loaded assemblies seems to be one of the things that behaves differently when deployed to Azure than when testing locally, so Type.GetType is something to avoid if possible.

jzabroski commented 4 years ago

Interesting. When you say Type.GetType, what are you referring to? Are you referring to the static method (as in here), or also the per-instance method?

antichaosdb commented 4 years ago

The static one as mentioned in the original issue. It might just be the "TypeName, AssemblyName" scenario that causes problems. As long as you're doing reflection on assemblies that are already loaded, Azure is fine. Loading at runtime by name is problematic on Azure because your files sometimes aren't where you or your code think they are.

jzabroski commented 4 years ago

Are those assemblies listed in your deps.json file? I am not an Azure architect or Microsoft employee, but if I were to design this, I would probably drive everything off of deps.json to ensure assembly loading works correctly.

jzabroski commented 4 years ago

@antichaosdb Take a look at my fix. It's a bit different from what you suggested. In my measurement, it doesn't significantly slow down performance to do it this way, and should prevent the problems you saw with ambient calls to Type.GetType().

I recommend filing a bug with Microsoft on this as well, and send them your log file. If you're paying for Azure Functions, you should get support here, as bugs like these waste the great efforts to make the CLR the most powerful web runtime on the planet. It is already super fast - now let's help make it super-stable! :)

jzabroski commented 4 years ago

@antichaosdb I pushed 2.0.0-beta7 which should fix this problem.