многопоточность: замок на собственность - это правильно?

Question

Я написал следующий код:

static readonly object failedTestLock = new object(); 

public static Dictionary<string, Exception> FailedTests 
{ 
    get 
    { 
     lock (failedTestLock) 
     { 
      return _failedTests; 
     } 
    } 
    set 
    { 
     lock (failedTestLock) 
     { 
      _failedTests = value; 
     } 
    } 
} 

public void RunTest(string testName) 
{ 
    try 
    { 
     //Run a test 
    } 
    catch (Exception exception) 
    { 
     // ?? Is this correct/threadsafe? 
     FailedTests.Add(testName, exception); 
    } 
} 

ВОПРОС:
Является ли это правильным способом, чтобы безопасно добавить отказавший тест словарю?
Является ли это потокобезопасным?
Is FailedTests.Add вызвал INSIDE блокировку или ЗАКРЫТЬ замок?

Можете ли вы объяснить, почему это правильно/threadsafe или почему нет?

Заранее спасибо

Answer 1

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

Это правильный способ безопасно добавить неудачный тест в словарь?

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

Это потоки?

It depends what you mean by threadsafe, но нет, не по разумному определению.

Is FailedTests.Add вызван ВНУТРИ замка или ЗАКРЫТЬ замок?

Поиск словаря (get accessor) происходит внутри замка. Этот код вызывает Add после освобождения блокировки.

Можете ли вы объяснить, почему это правильно/threadsafe или почему нет?

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

Answer 2

Это не поточно-доступ к словарю, потому что только доступ к свойству, которое возвращает объект словаря является поточно-безопасным, но вы не синхронизированным вызовом метода Add. Рассмотрите возможность использования ConcurrentDictionary<string,Exception> в этом случае или выполните синхронизацию вызовов до Add вручную.

Answer 3

Я не думаю, что это потокобезопасно, потому что замок хранится только в самый короткий момент, когда возвращается указатель на коллекцию. Когда вы добавляете в коллекцию, нет блокировки, поэтому, если два потока попытаются добавить одновременно, вы получите неприятную ошибку. Итак, вы должны заблокировать код FailedTest.Add.

Возможно, вы также захотите изучить параллельные коллекции, они могут предоставить то, что вам нужно.

С уважением GJ

Answer 4

Звонок на номер Add() находится за пределами замков.

Вы можете решить эту проблему, написав собственный метод Add() для замены свойства.