Stack Overflow with DistributedSession access inside custom ILogger

[almostchristian]

My custom ILogger implementation accesses the session object to retrieve and log the session ID, but the session has not been loaded yet and results in a recursion causing a StackOverflowException. The message during the recursion is 'Accessing expired session, Key:xxxx'. My custom logger uses IHttpContextAccessor to retrieve the HttpContext and ISession object.

The ISession is a DistributedSession instance, and from the code, we see that logging occurs before the isLoaded variable is set to true which results in a recursion when the _isNewSessionKey flag is initialized as false.

I was able to mitigate this error by using DataProtection with persisted keys.

services.AddDataProtection()
    .PersistKeysToFileSystem(new DirectoryInfo("keys"));

My app is netcoreapp2.0, but I see someone reported this same issue in StackOverflow 😄 back in 2016.

Is there an alternate way to retrieve the session ID from inside the logger?

[Tratcher]

Write a middleware that accesses session and stores the ID in HttpContext.Items, which Logging can pull from. If the id isn't there then you break the loop by not including it.

[almostchristian]

Using a middleware to save the sessionId works. Is this issue known? Seems like the fix is simple.

[Tratcher]

What fix do you propose?

[almostchristian]

The stackoverflow happens in the call of _logger.AccessingExpiredSession(_sessionKey), which causes Load() to be called again because the _loaded flag is only set in the finally clause. Setting the _loaded flag beforehand should fix the issue. I realize this solution doesn't follow the style in other code, but this is looks to be the simplest solution.

private void Load()
{
    if (!_loaded)
    {
        try
        {
            var data = _cache.Get(_sessionKey);
            if (data != null)
            {
                Deserialize(new MemoryStream(data));
            }
            else if (!_isNewSessionKey)
            {
                _loaded = true; //added to prevent StackOverflowException
                _logger.AccessingExpiredSession(_sessionKey);
            }
                _isAvailable = true;
            }
            catch (Exception exception)
            {
                _logger.SessionCacheReadException(_sessionKey, exception);
                _isAvailable = false;
                _sessionId = string.Empty;
                _sessionIdBytes = null;
                _store = new NoOpSessionStore();
            }
            finally
            {
                _loaded = true;

[shirhatti]

@Tratcher Can you help here? I don't have enough context

[Tratcher]

I don't recommend making any changes. Accessing statefull HttpContext fields inside the logger is dangerous and to be discouraged. App insights had similar issues and also fixed it by pre-reading any data they wanted at the start of the pipeline into their own data structure and then only having the logger touch that stable structure.

[analogrelay]

Closing as per @Tratcher 's comment.