Fix possible deadlock issue with AspNetLayoutRendererBase.Register

[snakefoot]

RetrieveHttpContextAccessor is a dangerous method, and should not be called frequently, because it can introduce deadlocks. See also: NLog/NLog#2064

Instead one should create a new class FuncAspNetLayoutRenderer, that can hold the func-delegate and perform caching of HttpContextAccessor.

[snakefoot]

Could at the same time suppress ObjectDisposedException when logging outside-request-scope. See also: #630

[304NotModified]

We need to change things in NLog for this.

• it should be possible to register FuncLayoutRenderer (or a like)
• FuncLayoutRenderer.RenderMethod method should be settable (protected) and a protected ctor without this method is needed.

Another option is to copy FuncLayoutRenderer, but then you're missing IStringValueRenderer for example and coping code is never really nice (e.g. FormatAsJson)

Also...

• I don't know why FuncLayoutRenderer.RenderMethod is public
• I don't know why FuncLayoutRenderer is public, but it's nice for this

[snakefoot]

I prefer just to make an internal class in Nlog.Web that takes the input method as constructor parameter.

[304NotModified]

Yes, but we still need the changes in NLog itself ;)

[snakefoot]

Why?

[snakefoot]

Maybe something like this (if you think FunctionLayoutRenderer is the best thing):

internal class NLogWebLayoutRenderer : FunctionLayoutRenderer
{
     Func<LogEventInfo, HttpContextBase, LoggingConfiguration, object> _func;

     private IHttpContextAccessor HttpContextAccessor { get; set; }   // TODO Missing implementation

     public NLogWebLayoutRenderer(string name, Func<LogEventInfo, HttpContextBase, LoggingConfiguration, object> func)
         :base(name, (event, config) => LookupValue(event, config))
     {
         _func = func;
     }

     private object LookupValue(LogEventInfo logEvent, LoggingConfiguration config)
     {
          var httpContext = HttpContextAccessor?.HttpContext;
          return _func(logEvent, httpContext, config);
     }
}

Alternative one could have FunctionLayoutRenderer as a member an initialize it when needing it, and instead inherit from AspNetLayoutRendererBase (And get HttpContextAccessor for free)

[304NotModified]

I will look into this :)

[304NotModified]

LookupValue needs to be static then (and so also HttpContextAccessor property). And I don't think we like static "cache"

If static is ok, we could add it to the AspNetLayoutRendererBase. But that doesn't look right

[snakefoot]

Then I guess FunctionLayoutRenderer has to be improved, by adding a new protected virtual method called RenderValue() that returns object (Think messing with RenderMethod-property is a mistake).

And at some point this special implementation of FunctionLayoutRenderer will use Resolve<T> to lookup the HttpContext-accessor.

[304NotModified]

yes indeed, see NLog/NLog#4326 :)

[304NotModified]

working on this --> #659