ICollection, коллекции только для чтения и синхронизация. Это правильно? - PullRequest
6 голосов
/ 17 октября 2008

У меня есть собственный класс, который реализует ICollection, и этот класс доступен только для чтения, т.е. IsReadOnly возвращает true (в отличие от использования ключевого слова readonly), и все функции, которые обычно изменяют данные в коллекции, выдают InvalidOperationException.

Теперь, учитывая такую ​​конструкцию и быстрое рассмотрение проблем безопасности потоков при реализации ICollection (в частности, ICollection.IsSynchronized и друзей), я нашел это быстрое и грязное решение.

bool ICollection.IsSynchronised { get{ return true; } }
object ICollection.SyncRoot { get{ return new Object(); } }

Теперь, учитывая примеры в MSDN, это не приведет к правильной блокировке разных потоков, потому что они получают разные объекты из SyncRoot. Учитывая, что это только для чтения коллекция, это проблема? Есть ли проблемы с памятью и сборкой мусора при возврате new Object()? Любые другие проблемы, которые вы можете увидеть с этой реализацией?

Ответы [ 3 ]

4 голосов
/ 17 октября 2008

Да, это проблема в некоторых случаях. Даже если коллекция доступна только для чтения и не может быть изменена, объекты, на которые ссылается коллекция, не только для чтения. Таким образом, если клиенты используют SyncRoot для выполнения блокировки, они не будут поточно-ориентированными при изменении объектов, на которые ссылается коллекция.

Я бы порекомендовал добавить:

private readonly object syncRoot = new object();

в ваш класс. Верните это как SyncRoot, и все готово.

2 голосов
/ 17 октября 2008

Кажется очень странным каждый раз возвращать разные объекты ... на самом деле, я очень редко (если вообще когда-либо) использую подход SyncRoot, так как часто мне нужно синхронизировать несколько объектов и т. Д., Поэтому отдельный объект более осмыслен.

Но если данные действительно неизменны (только для чтения), почему бы просто не вернуть false из IsSynchronized?

Re GC - любой такой объект обычно будет недолговечным и будет собран в GEN0. Если у вас есть поле с объектом (для блокировки), оно будет длиться столько же, сколько и коллекция, но, скорее всего, не повредит ...

Если вам нужен замок, я бы просто соблазнил:

private readonly object lockObj = new object();

Вы также можете использовать ленивый подход только для «нового» его при необходимости, что имеет определенный смысл, если вы на самом деле не ожидаете, что кто-либо попросит синхронизировать блокировку (возвращая false для IsSynchronized).

Другой распространенный подход - вернуть «это»; это делает вещи простыми, но рискует конфликтовать с некоторым другим кодом, использующим ваш объект в качестве блокировки для несвязанной цели. Редко, но возможно. Это именно тот подход, который [MethodImpl] использует для синхронизации.

2 голосов
/ 17 октября 2008

Полагаю, проблема заключалась бы в том, чтобы клиенты использовали корневой каталог синхронизации для блокировки не только вашей коллекции, но и чего-то еще. Предположим, что они кэшировали размер коллекции - или, может быть, «какое подмножество этой коллекции соответствует предикату» - они разумно предположили бы, что могут использовать ваш SyncRoot для защиты как вашей коллекции, так и другого члена.

Лично я почти не использую SyncRoot, но думаю, что было бы разумно всегда возвращать одно и то же.

...