4

Коллеги, помогите с кодом.

Пишу метод получения кастомного набора словарей из БД, который должен вызвать ряд асинхронных методов, дождаться выполнения каждого из них, сформировать "словарь словарей" и вернуть результат. Сигнатура метода следующая:

public async Task<IDictionary<LookupType, IEnumerable<RacetrackLookupBase>>> GetLookups(IEnumerable<LookupType> requiredTypes)

Попытка 1. Запустить сразу кучу тасков, тянущих данные из БД, положить в IEnumerable, сделать await WhenAll() и сформировать словарь из результатов тасков провалилась. EF не дает запустить вторую асинхронную операцию, используя тот же контекст данных, в котором запущена первая.

Попытка 2

return requiredTypes
            .Select(async t =>
            {
                IEnumerable<RacetrackLookupBase> lookup = await GetLookup(t);
                return new KeyValuePair<LookupType, IEnumerable<RacetrackLookupBase>>(t, lookup);
            })
            .ToDictionary(item => item.Result.Key, item => item.Result.Value);

Не собирается, получаю ошибку "Contract extraction failed: async / iterator issue:". Почитал что есть такая трабла с контрактами, если cделать async метод который не использует await (хотя await вроде как используется внутри Select).

Попытка 3 Оставил почти тот же код. Только в конце костыль прилепил чтобы await "официально" присутствовал в методе

var dictionary = requiredTypes
            .Select(async t =>
            {
                IEnumerable<RacetrackLookupBase> lookup = await GetLookup(t);
                return new KeyValuePair<LookupType, IEnumerable<RacetrackLookupBase>>(t, lookup);
            })
            .ToDictionary(item => item.Result.Key, item => item.Result.Value);

    return await Task.FromResult(dictionary);

Получаю дедлок в момент вызова Select(). Поток зависает навсегда.

Вариант 4 Убрал LINQ и сделал по простому. Все работает

var lookups = new Dictionary<LookupType, IEnumerable<RacetrackLookupBase>>();
        foreach (var type in requiredTypes)
        {
            var lookup = await GetLookup(type);
            lookups.Add(type, lookup);
        }

return lookups;

Метод GetLookup() описан следующим образом

 public async Task<IEnumerable<RacetrackLookupBase>> GetLookup(LookupType type)
    {
        switch (type)
        {
            case LookupType.AgeCategory:
                return await _db.LookupAgeCategories.ToListAsync();
            case LookupType.Distance:
                return await _db.LookupDistances.ToListAsync();
            ...
            default:
                return await Task.FromResult(new List<RacetrackLookupBase>());
        }
    }

Поясните плз, какие ошибки были допущены в вариантах? Какое решение наилучшее?

andreycha
  • 25,167
  • 4
  • 46
  • 82
nikita
  • 1,025
  • А что делает GetLookup? Там есть обращение к базе? – VladD Jun 21 '16 at 13:26
  • именно. Используя асинхронный интерфейс EF. Типа ToListAsync() – nikita Jun 21 '16 at 13:29
  • Окей, а какого типа requiredTypes? Это IEnumerable просто, да? – VladD Jun 21 '16 at 13:33
  • да, все верно. Сигнатура метода следующая public async Task<IDictionary<LookupType, IEnumerable<RacetrackLookupBase>>> GetLookups(IEnumerable<LookupType> requiredTypes) – nikita Jun 21 '16 at 13:37
  • Окей, сейчас напишу. – VladD Jun 21 '16 at 13:41

2 Answers2

5

Имхо, но есть пара советов (1) из идеалогии EF, если версия выше 4 и (2) по личному опыту:

  1. Создавать контексты EF на каждую операцию. Если 6.0+ то вообще рекомендую. Они весьма легковесные, и это не будет тормозить, по сравнению с материализацией запроса.
  2. Асинхронные вызовы оборачивать в отдельные классы-контексты операций (в вашем случае можно 1 generic, как понимаю), так будет легче отлаживаться. Т.е. если есть уникальный запрос, то лучше весь асинхронный код объединить в контекст и вынести в класс-помошник, тем самым создав контроль над замыкаемыми объектами.

т.е. первый вариант был норм, только с созданным внутри таска контекстом, который создается-материализует-умирает в рамках отдельного потока.

  1. (Дополенение) Это не решение, что бы не было неясностей. Это просто шаг в сторону single responsobility. Т.е. область ответственности за асинхронный вызов, разделение доступа к ресурсам несет отдельный логический элемент + никто не засунет в лямду какое-то поле "случайно" и не сделает его критической секцией без lock-а.Например, есть у тебя потребность в асинхронном выполнении запроса, который на вход получает object t. Понятно, что t становится разделяемым ресурсом между потоком вызова и таска. Так что важно обеспечить неизменяемость на время выполнения задания. Но в целом, будем считать что так и есть, и интерфейс блокирует модификацию объекта t "сверху". Так что перейдем к реализации задачи.
return requiredTypes
            .Select(async t =>
            {
                IEnumerable<RacetrackLookupBase> lookup = await     GetLookup(t);
                return new KeyValuePair<LookupType, IEnumerable<RacetrackLookupBase>>(t, lookup);
            })
            .ToDictionary(item => item.Result.Key, item => item.Result.Value);

Проблема в данном случае в том, что 1. В лямду можно запихнуть что угодно из внутренностей класса. 2. GetLookup - метод класса. Пускай и приватный, но в целом живой и имеет доступ ко всем полям внутри класса и если разработчик обратиться к полю - получаем неконтролируемое разрастание критической секции и кучу багла на пустом месте.

Что предлагается сделать. Определить класс-контекст. К примеру,

public class SomeAsyncContext:IDisposable
{
//в конструктор явно помещаем все, что необходимо для вызова асинхронного метода. Именно эти "объекты" и будут нашими разделяемыми ресурсами. При отладке ты сразу увидишь, что было замкнуто и легче определить сбой, имхо.
public SomeAsyncContext(object t, int a,int b)
{
  _t=t; _a=a;_b=b;
}

//Входных параметров нет, т.к. все уже внутри. Контекст легкий и на 1 операцию создается всякий раз новый и нет пока нужды параметры перезадавать, копя возможную ошибку состояния.
pubic async Task<Dictionary<object,object>> DoSome()
{
///делаем наши асинхронные дела. 
///Можно и просто return await GetLookup(_t);
///но в этом случае это будет уже внутренний метод и в единичном экземпляре.

}

Я бы сразу в лямде тело развернул. Получается, что мы не мешаем синхронный код с асинхронным, ставя вот такой логический "барьер" между ними. Опять же, это мой опыт, т.е. ИМХО. Использование выглядит просто

using(var context = new SomeAsyncContext(t,a,b))
{
   result = await context.DoSome();
}

вот как-то так. И еще накинуть конрактов в контекст. Что бы сделать неизменяемыми внутри класса параметры.

Arheus
  • 548
  • 1
    Спасибо за советы.

    наша задача разрабатывается не первый день, поэтому трудно будет резко перейти на ваш вариант1. Уже есть куча оберток и классов , использующих 1 контекст, инжектируемый в конструктор. В принципе переписать можно, но надо все обдумать. А можете приложить пример кода для Вашего вариант2.

    – nikita Jun 21 '16 at 14:03
  • Всмысле дать пример оборачивания в контекст операции? – Arheus Jun 21 '16 at 14:18
  • да, я не совсем понял Ваш вариант2. можно псевдокодом – nikita Jun 22 '16 at 06:59
  • 1
    @nikita дополнил текст. – Arheus Jun 22 '16 at 07:19
4

Окей, смотрите. У нас есть ограничение, что методы GetLookup нужно вызывать параллельно. От него и будем плясать.

Попытка 1 — так делать оказалось нельзя, запускать все Task'и одновременно не катит. Окей.

Попытка 2. Здесь давайте посмотрим, что произошло после первого Select'а. Отвлечёмся на секундочку от того, что на вас ругаются Code Contracts. У вас по сути получился ленивый IEnumerable тасков. Затем вы пытаетесь применить к ним ToDictionary. Это синхронный метод, поэтому он пробежит ваш IEnumerable, создаст таски, и вызовет на них Result. Если создание тасков по идее может оказаться строго последовательным (хотя гарантии нет), то уж вызов Result — плохо, вы дожидаетесь результата синхронно, блокируя текущий поток.

Попытка 3. Та же проблема, await Task.FromResult(dictionary) ничего не меняет.

Вариант 4. Ну да, правильно.

Смотрите, если вы хотите использовать LINQ, вам нужно по сути асинхронный WhenAll, но который обязательно запускает пробегает свой список последовательно. Например, такое:

static class TaskEx
{
    public async static Task<T[]> WhenAllSequential<T>(IEnumerable<Task<T>> tasks)
    {
        var results = new List<T>();
        foreach (var t in tasks)
            results.Add(await t);
        return results.ToArray();
    }
}
VladD
  • 206,799
  • Супер, спасибо. Варианты 2 и 3 теперь стали более понятными для меня. Однако до сих пор не совсем ясно почему в варианте 3 поток уходил в бесконечное ожидание. Даже если мы ждали результат синхронно, он должен был когда-то вернуться. – nikita Jun 21 '16 at 14:05
  • @nikita: Мне сложно сказать, не видя подробностей функции GetLookup. Ил может быть был race condition между различными Task'ами. Интересно было бы взглянуть на stack trace в момент бесконечного ожидания. – VladD Jun 21 '16 at 14:11
  • добавил к вопросу код метода GetLookup(). Уточню, что бесконечное ожидание в варианте3 стартует сразу после первого вызова item => item.Result.Key – nikita Jun 22 '16 at 06:58
  • 2
    @nikita Вот почему бесконечное ожидание при вызове Result случается: http://ru.stackoverflow.com/questions/514529/%D0%97%D0%B0%D0%B2%D0%B8%D1%81%D0%B0%D0%B5%D1%82-%D0%BE%D0%BF%D0%B5%D1%80%D0%B0%D1%82%D0%BE%D1%80-await-%D0%B2-%D0%BE%D0%BA%D0%BE%D0%BD%D0%BD%D0%BE%D0%BC-%D0%BF%D1%80%D0%B8%D0%BB%D0%BE%D0%B6%D0%B5%D0%BD%D0%B8%D0%B8-%D0%BF%D1%80%D0%BE%D0%B3%D1%80%D0%B0%D0%BC%D0%BC%D0%B0-%D0%B2%D0%B8%D1%81%D0%B8%D1%82-%D0%BF%D1%80%D0%B8-%D0%B2%D1%8B%D0%B7%D0%BE%D0%B2%D0%B5-task – Pavel Mayorov Jun 22 '16 at 07:28
  • @PavelMayorov т.е. ConfigureAwait(false) решит мою проблему? – nikita Jun 23 '16 at 07:57
  • @nikita: А почему бы не попробовать? Да, в некоторых случаях добавление ConfigureAwait(false) помогает. – VladD Jun 23 '16 at 08:11
  • @nikita да, решит. Но VladD привел вам более аккуратное решение. – Pavel Mayorov Jun 23 '16 at 09:27