Обзор заблокированной части функции в C # .NET

Question

Является ли этот фрагмент кода, где я блокирую часть функции правильно? Или он может использовать недостатки, когда несколько сеансов одновременно запрашивают один и тот же экзамен?

Цель состоит в том, чтобы клиент, который первым запросит экзамен, собрал его, все последующие клиенты получат кэшированную версию.

public Exam GetExamByExamDto(ExamDTO examDto, int languageId)
{
    Log.Warn("GetExamByExamDto");
    lock (LockString)
    {
        if (!ContainsExam(examDto.id, languageId))
        {
            Log.Warn("Assembling ExamDto");
            var examAssembler = new ExamAssembler();
            var exam = examAssembler.createExam(examDto);

            if (AddToCache(exam))
            {
                _examDictionary.Add(examDto.id + "_" + languageId, exam);
            }
            Log.Warn("Returning non cached ExamDto");
            return exam;
        }
    }
    Log.Warn("Returning cached ExamDto");
    return _examDictionary[examDto.id + "_" + languageId];
}

У меня такое ощущение, что это не способ сделать это.

Answer 1

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

Просто используйте новый object в качестве блокировки:

private readonly object Padlock = new object();

См. это сообщение в блоге Тесс Феррандез.

Answer 2

Это выглядит в принципе нормально, но вы не должны блокировать string. Стажировка затрудняет контроль, какой экземпляр какой. Просто создайте отдельный объект для блокировки:

 private object padLock = new object();

Answer 3

Переменная LockString, это действительно строка? Не следует блокировать строку, так как вы можете столкнуться с проблемами, связанными с интернированием строк, которые приведут к блокировке нескольких блокировок.

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

Answer 4

Также вы можете использовать двойную проверку (после кэширования экзамена Monitor.Lock вообще не будет вызываться):

public Exam GetExamByExamDto(ExamDTO examDto, int languageId)
{
    Log.Warn("GetExamByExamDto");
    if (!ContainsExam(examDto.id, languageId))
    {
        lock (LockString) // Here should be some private locking object.
        {
            if (!ContainsExam(examDto.id, languageId))
            {
                Log.Warn("Assembling ExamDto");
                var examAssembler = new ExamAssembler();
                var exam = examAssembler.createExam(examDto);

                if (AddToCache(exam))
                {
                    _examDictionary.Add(examDto.id + "_" + languageId, exam);
                }
                Log.Warn("Returning non cached ExamDto");
                return exam;
            }
        }
        Log.Warn("Returning cached ExamDto");
        return _examDictionary[examDto.id + "_" + languageId];
    }
}