对 .NET 中的静态成员的并发访问答案

【问题标题】：Concurrent acces to a static member in .NET对 .NET 中的静态成员的并发访问
【发布时间】：2011-07-17 10:27:39
【问题描述】：

我有一个包含静态集合的类，用于将登录用户存储在 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);
        }
    }
}

【问题讨论】：

顺便说一句 - 这些代码都不是异步的......问题标题令人困惑
我认为这些成员不应该是静态的。你背后的原因是什么？
@Marc，你是对的。异步与此无关。当我输入问题时，我有点搞砸了:)

标签： c# .net multithreading concurrency locking

【解决方案1】：

这绝对不是线程安全的。任何时候 2 个线程在做某事（在 Web 应用程序中很常见），都可能出现混乱 - 异常或静默数据丢失。

是的，您需要某种同步，例如lock；而static 通常对于数据存储来说是一个非常糟糕的主意，IMO（除非非常小心地处理并仅限于配置数据之类的东西）。

另外 - static 事件以一种让对象图意外保持活力的好方法而臭名昭著。对待那些也要小心；如果您只订阅一次，很好 - 但不要根据请求订阅等。

另外 - 它不仅仅是锁定操作，因为这一行：

return onlineUsers;

返回您的列表，现在不受保护。所有对项目的访问必须同步。我个人会返回一个副本，即

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

最后，从此类返回 .Count 可能会令人困惑 - 因为不能保证在任何时候它仍然是 Count。 仅在那个时间点提供信息。

【讨论】：

我知道你得到了很多，但是很好的答案！
您关于“计数”不可靠的观点可能是可变类型的多线程编程中最重要且经常被误解的方面。通常你在单线程生活中推理代码的方式是内存是稳定的，除非你努力改变它。在多线程代码中，情况正好相反：您必须假设一切都在不断变化，除非您正在努力使其保持稳定。当您观察集合的大小时您正在观察它过去的大小。数一数之后，它可能立即变异了一百万次。
@Eric 我认为微妙的是不小心暴露了名单，现在完全没有保护，赤身裸体，容易受到攻击。
@Marc 我同意约翰的观点，你给出了一个很好的答案。我不明白为什么静态事件不好。您能否指出一些链接或参考资料以了解更多信息。
@Mark 对待他们很好；但是如果您订阅了一个 transient 对象（它的生命周期很短），但 忘记取消订阅，那么您将永远保持该对象的生命周期.当您重复该过程时，它会导致内存随着时间的推移而增长，并且会使调用它的速度越来越慢。

【解决方案2】：

是的，您需要锁定 onlineUsers 以使该代码线程安全。

几点说明：

使用HashSet<string> 代替List<string> 可能是个好主意，因为这样的操作效率更高（尤其是Contains 和Remove）。不过，这不会改变锁定要求的任何内容。
如果类只有静态成员，您可以将其声明为“静态”。

【讨论】：

他可以将其设为静态类，但 IMO 这些成员一开始就不应该是静态的。
@CodeInChaos，这当然是对的。但是由于 Op 显然是一个新手，我不想写一个“你做错了”之类的答案。在这里替换静态类的合适技术对于 Op 来说可能有点复杂。
@CodeInChaos 你能告诉我为什么这些成员不应该被标记为静态。我只是想纠正我的错误。
@Lucero 单个锁对象是否足以在所有地方锁定集合？
@Mark，除了GetUsers 类，单个锁就足够了。正如其他人所写，您不应返回 GetUsers 中的原始实例，否则您将无法控制对该引用执行的操作。

【解决方案3】：

是的，您确实需要锁定您的代码。

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

【讨论】：

【解决方案4】：

是的。您需要在读取或写入集合之前锁定集合，因为可能会从不同的线程池工作人员添加多个用户。您可能也应该在计数上执行此操作，但如果您不关心 100% 的准确性，这可能不是问题。

【讨论】：

【解决方案5】：

根据 Lucero 的回答，您需要锁定 onlineUsers。还要注意您班级的客户将如何处理从 GetUsers() 返回的 onlineUsers。我建议您更改您的界面 - 例如使用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() 的结果，则此实现可能会使您陷入死锁。

【讨论】：

这将导致阻塞，而不是通常的死锁（除非有第二个资源在玩） - 但我个人会避免这种情况 - 返回数据副本通常非常便宜并且避免了巨大的风险与这种方法相关联。特别是，我可以预见像foreach(var user user in GetUsers()) DoSomethingExpensive(user) 这样的东西会在不可预测（但太长）的时间内阻止所有其他访问。最好避免，IMO。
@Marc 同意，但这取决于具体的使用场景。如果用户较少且列表经常修改，则副本可能会更好。如果有很多用户并且列表相当静态（并且您的示例中没有 DoSomethingExpensive() ），那么我的解决方案可能会更好。一如既往地“取决于”TM ;)
重新删除评论重新死锁；不，不会死锁 - 锁是可重入的。是的，您确实需要锁定 .Count
@Marc 如果遍历 GetUsers() 的线程尝试等待另一个线程，然后尝试获取列表上的锁，这将导致死锁。
在线程上等待 - 我称之为资源 :) 但是是的，that 会死锁。所以如果这是一个真正的风险 - 不要像这样暴露数据:)

【解决方案6】：

该代码本身不是线程安全的。

我不会对您的“设计”提出任何建议，因为您没有提出任何建议。我将假设您找到了这些静态成员的充分理由并像您一样公开您的列表内容。

然而，如果你想让你的代码线程安全，你基本上应该使用一个锁对象来锁定，并用一个锁语句包装你的方法的内容：

    private readonly object syncObject = new object();

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

请注意，引发的这些事件可能会长时间持有锁定，具体取决于代表的操作。您可以省略 NoOfOnlineUsers 属性中的锁定，同时将您的列表声明为 volatile。但是，如果您希望 Count 值在您在某个时刻使用它时一直保持不变，那么也可以在此处使用锁。

正如其他人在此建议的那样，直接公开您的列表，即使有锁，仍然会对其内容构成“威胁”。按照 Mark Gravell 的建议，我会退回一份副本（这应该适合大多数用途）。

现在，既然您说您在 ASP.NET 环境中使用它，那么值得一提的是，所有局部变量和成员变量，以及它们的成员变量（如果有的话）都是线程安全的。

【讨论】：