Сбор контекстной информации для логирования

[@naa]

А какой смысл там в lock'ах и ConcurrentBag'е? Если используется только async/await и потоки не создаются «руками», то как раз AsyncLocal.Value исключает конкурентный доступ в рамках одной цепочки задач (Task'ов).

[@iakimov]

На самом деле сервис может одновременно запустить несколько потоков и ожидать их завершения с помощью Task.WhenAll. В этом случае несколько потоков могут обращаться к одному и тому же экземпляру таймера или ErrorContext. Поэтому необходимость в ConcurrentBag есть.

Действительно ли так нужен lock — вопрос. На всякий случай я добавил его.

[@naa]

Если речь про одновременные запросы в разных потоках, то они будут запущены все равно в рамках разных задач (Task) и контекст синхронизации будет разный и у каждой задачи опять будет свой AsyncLocal.Value.
Инстанст ConcurrentBag он свой на каждый запрос, а внутри async/await не может быть одновременно несколько рабочих потоков, которые будут конкурировать за ConcurrentBag. Даже если после await продолжение задачи (Task) попадет на другой поток из пула, она будет выполнена последовательно относительно других задач.

[@iakimov]

Речь идет о создании разных потоков в рамках одного запроса к сервису. Что-то типа:

ErrorContext.CreateNewErrorContext();

Task[] tasks = Enumerable.Range(1, 10).Select(i => Task.Factory.StartNew(() =>
{
    // do something

    ErrorContext.Current.AttachMessage("message");

    // do another thing
})).ToArray();

await Task.WhenAll(tasks);

В этом случае все запущенные задачи будут одновременно обращаться к одному и тому же экземпляру ErrorContext.

Если же все такие задачи последовательно ждутся с помощью await, то доступ к экземпляру ErrorContext будет действительно последовательный, а не одновременный:

ErrorContext.CreateNewErrorContext();

await Task.Factory.StartNew(() =>
{
    // do something

    ErrorContext.Current.AttachMessage("message");

    // do another thing
});

await Task.Factory.StartNew(() =>
{
    // do something

    ErrorContext.Current.AttachMessage("message");

    // do another thing
});

// waiting for other tasks

[@naa]

А в пером примере не будет разве для каждой созданной задачи свой AsyncLocal.Value? Вот такой пример

static AsyncLocal<long> asyncCounter = new AsyncLocal<long>();

static void Main(string[] args)
{
    Task[] tasks = Enumerable.Range(1, 10).Select(i => Task.Factory.StartNew(() =>
    {
        asyncCounter.Value++;
    })).ToArray();

    Task.WhenAll(tasks).GetAwaiter().GetResult();

    Console.WriteLine(asyncCounter.Value);
}

ожидаем выведет на консоль 0

[@AlexZyl]

It depends.

AsyncLocal-данные хранятся в словаре _localValues ExecutionContext-а. Т.к. при создании таски через StartNew происходит захват контекста, то «дочерний» контекст получает все данные «родительского». А так как автор использует обертку для данных в виде ErrorContext, то два ExecutionContext-а шарят референс на инстанс ErrorContext-а. Так что ConcurrentBag вполне уместен.

В вашем примере, значение остается нулевым, т.к. референс на parentEc._localValues != childEc._localValues, а значит, что при инкременте замена объекта в _localValues внутри созданной таски происходит только для childEc._localValues.

А вот почему ссылки словарей различаются я пока не понял. Словно производится копирование содержимого родительского _localValues в дочерний, хотя по коду ExecutionContext.Capture копирует лишь ссылку на словарь.

[@AlexZyl]

Проглядел участок кода, где на Set в ExecutionContext-е происходит создание нового словаря с копированием содержимого. Ну а раз у нас одно из значений — long…

[@AlexZyl]

Пример кода:

public class CounterContext
{
    public long AsyncCounter;
}

static AsyncLocal<CounterContext> CounterContext = new AsyncLocal<CounterContext>();

static void Main(string[] args)
{
    CounterContext.Value = new CounterContext();
    Task[] tasks = Enumerable.Range(1, 1000).Select(i => Task.Factory.StartNew(() =>
    {
        CounterContext.Value.AsyncCounter++;
    })).ToArray();

    Task.WhenAll(tasks).GetAwaiter().GetResult();

    Console.WriteLine(CounterContext.Value.AsyncCounter);
}

Выведет 997-1000 из-за конкурентного доступа

[@AlexZyl]

Что-то подсказывает, что такая инициализация контекста в фильтре может ударить по перфомансу при нагрузке.
Не лучше ли вынести этот код в какой-нибудь ErrorContextInitializationMiddleware и зарегистрировать на PreHandlerExecute стэйдж? Тогда синхронизации доступа к Current контексту не понадобится, а логирование вынести в кастомный IExceptionLogger (раз уж речь в статье идет о ASP.NET WebAPI)

[@iakimov]

Вполне возможно, что использование middleware для инициализации контекстов будет более эффективно. Хотя вроде бы никакой существенной работы при этом не делается. Но, по хорошему, надо, конечно, проверять.

[@qw1]

IIS использует пул потоков, и у меня были случаи при работе с NHibernate, когда запрос начался на одном потоке, а закончился на другом. То есть, когда требуется ожидание данных из БД, IIS не блокирует поток, а отдаёт его другому запросу. Используя [ThreadStatic] переменную для контекста, я получал полную кашу (в начале метода её инициализирую, а на после чтений из БД получаю значение от другого запроса). Тут не будет такой проблемы?

Специально для хранения пользовательских переменных и получения к ним доступа в static-контекстах есть словарик HttpContext.Current, чтобы не изобретать велосипеды.

[@AlexZyl]

Используя [ThreadStatic] переменную для контекста, я получал полную кашу (в начале метода её инициализирую, а на после чтений из БД получаю значение от другого запроса).

С MSDN

Indicates that the value of a static field is unique for each thread.

Так что вполне ожидаемое поведение

ExecutionContext же не связан с конкретным потоком, а логически завязан на поток исполнения.

Специально для хранения пользовательских переменных и получения к ним доступа в static-контекстах есть словарик HttpContext.Current, чтобы не изобретать велосипеды.

Мир не одним Web-ом един. Для Presentation-слоя это может и сойдет, но вы же не будете тянуть System.Web в DAL или консольное приложение?

[@qw1]

ExecutionContext же не связан с конкретным потоком, а логически завязан на поток исполнения.

Но знает ли этот класс о переключении native threads в IIS? Некоторые авторы имеют опыт, что нет.

вы же не будете тянуть System.Web

Не напрямую, а через сборку-библиотеку, где расположен код логирования. Что поделать, другого надёжного решения нет.

или консольное приложение?

Не под IIS HttpContext.Current имеет значение null, поэтому нужен fall-back на другой механизм (тот же ThreadStatic, у если нас нет async)