toddams / RazorLight

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

Documentation: is CompileRenderStringAsync thread safe? #313

Closed epsitec closed 4 years ago

epsitec commented 4 years ago

More generally, I did not find any information about thread safety. Can I call CompileRenderStringAsync() on the same instance from different threads? Is this expected to work?

jzabroski commented 4 years ago

Have you found any behavior suggesting its not? Someone recently asked me about thread safety and I posted a retrospective analysis of thread safeness. Not sure which Issue I posted it in.

epsitec commented 4 years ago

No, I was wondering if I could allocate a single RazorLightEngineBuilder and reuse it throughout my code by calling CompileRenderStringAsync() from various threads, or if this was not considered safe.

jzabroski commented 4 years ago

I believe that should be safe. Will try to add some tests this week to verify. There is already one concurrency test case but its not super robust.

jzabroski commented 4 years ago

Actually, you may run into issues with #287 with fix proposed in #310

epsitec commented 4 years ago

Pull request https://github.com/toddams/RazorLight/pull/310 makes perfect sense. Such a breaking change might be a pain for current users, but I feel it should be implemented in the official release.

wgv-dzhang commented 4 years ago

Running into a similar issue here. Are there any plans to pull in pull request #310 ?

jzabroski commented 4 years ago

I ran into some technical problems merging it - I was using GitHub Desktop which has some nasty bugs that I reported and then proceeded to get banned from github.com/Desktop/Desktop for being a pain to them about it being totally broken.

Plus, just got back from vacation.

Anyway, tl;dr: I will try to get this merged sooner than later. My current client is actually upgrading to ASP.NET Core 3.1 now so there is real economics for me to fix this stuff.

jzabroski commented 4 years ago

Fixed by #310