Одновременный доступ к статическому члену в .NET

Question

У меня есть класс, который содержит статическую коллекцию для хранения вошедших в систему пользователей в приложении ASP.NET MVC. Я просто хочу знать, что приведенный ниже код является поточно-ориентированным или нет. Нужно ли блокировать код всякий раз, когда я добавляю или удаляю элемент в коллекцию onlineUsers.

public class OnlineUsers
{
    private static List<string> onlineUsers = new List<string>();
    public static EventHandler<string> OnUserAdded;
    public static EventHandler<string> OnUserRemoved;

    private OnlineUsers()
    {
    }

    static OnlineUsers()
    {
    }

    public static int NoOfOnlineUsers
    {
        get
        {
            return onlineUsers.Count;
        }
    }

    public static List<string> GetUsers()
    {
        return onlineUsers;
    }

    public static void AddUser(string userName)
    {
        if (!onlineUsers.Contains(userName))
        {
            onlineUsers.Add(userName);

            if (OnUserAdded != null)
                OnUserAdded(null, userName);
        }
    }

    public static void RemoveUser(string userName)
    {
        if (onlineUsers.Contains(userName))
        {
            onlineUsers.Remove(userName);

            if (OnUserRemoved != null)
                OnUserRemoved(null, userName);
        }
    }
}

Answer 1

Это абсолютно не потокобезопасно.Каждый раз, когда 2 потока что-то делают (очень часто в веб-приложении), возможен хаос - исключения или тихая потеря данных.

Да, вам нужна какая-то синхронизация, такая как lock;и static, как правило, очень плохая идея для хранения данных, IMO (если не рассматривать очень осторожно и не ограничиваться такими вещами, как данные конфигурации).

Также - static события печально известны хорошим способом сохранить объектГрафики живы неожиданно.Относитесь к тем же с осторожностью;если вы подписываетесь только один раз, хорошо - но не подписывайтесь и т. д. на запрос.

Кроме того - это не просто блокировка операций, так как эта строка:

return onlineUsers;

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

lock(syncObj) {
    return onlineUsers.ToArray();
}

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

Answer 2

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

Несколько заметок:

• Использование HashSet<string> вместо List<string> может быть хорошей идеей, поскольку оно намного более эффективно для подобных операций (особенно Contains и Remove). Это ничего не меняет в требованиях к блокировке.

• Вы можете объявить класс как "статический", если он имеет только статические члены.

Answer 3

Этот код сам по себе не ориентирован на многопотоковое исполнение.

Я не буду делать никаких предложений относительно вашего «дизайна», так как вы его не спрашивали.Я предположу, что вы нашли веские причины для этих статических членов и показали содержание своего списка, как вы это сделали.

Однако, если вы хотите сделать свой код поточно-ориентированным, вам следует в основном использоватьобъект блокировки для блокировки и оберните содержимое ваших методов оператором блокировки:

    private readonly object syncObject = new object();

    void SomeMethod()
    {
        lock (this.syncObject)
        {
            // Work with your list here
        }
    }

Остерегайтесь того, что вызываемые события могут удерживать блокировку в течение длительного периода времени, в зависимости отчто делают делегаты.Вы можете опустить блокировку из свойства NoOfOnlineUsers, объявив свой список как изменчивый.Тем не менее, если вы хотите, чтобы значение Count сохранялось до тех пор, пока вы используете его в определенный момент, используйте также блокировку там.

Как и другие здесь предлагают, выставляя ваш список напрямую, даже сблокировка, все равно будет представлять «угрозу» для его содержимого.Я бы пошел с возвратом копии (и это должно соответствовать большинству целей), как советовал Марк Гравелл.

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

Answer 4

Согласно ответу Лусеро, вам нужно заблокировать онлайн-пользователей. Также будьте внимательны, что будут делать клиенты вашего класса с onlineUsers, возвращаемыми из GetUsers (). Я предлагаю вам изменить свой интерфейс - например, используйте IEnumerable<string> GetUsers() и убедитесь, что блокировка используется в его реализации. Как то так:

public static IEnumerable<string> GetUsers() {
    lock (...) {
        foreach (var element in onlineUsers)
            yield return element;
        // We need foreach, just "return onlineUsers" would release the lock too early!
    }
}

Обратите внимание, что эта реализация может выставить вас в тупики, если пользователи попытаются вызвать какой-либо другой метод OnlineUsers, который использует блокировку, в то же время повторяя результаты GetUsers ().

Answer 5

Да. Вам нужно заблокировать коллекцию, прежде чем читать или писать в коллекцию, так как несколько пользователей потенциально могут быть добавлены из разных работников пула потоков. Вероятно, вам следует также делать это и на счету, хотя, если вас не интересует 100% точность, это не проблема.

Answer 6

Да, вам нужно заблокировать свой код.

 object padlock = new object
 public bool Contains(T item)
 {
    lock (padlock)
    {
        return items.Contains(item);
    }
 }