我有什么选择来使这个代码线程安全？

Question

我有这段代码,为了简洁省略了很多东西,但场景是这样的:

 public class Billing
    {
        private List<PrecalculateValue> Values = new List<PrecalculateValue>();

        public int GetValue(DateTime date)
        {
            var preCalculated = Values.SingleOrDefault(g => g.date == date).value;
            //if exist in Values, return it
            if(preCalculated != null)
            {
               return preCalculated;
            }

            // if it does not exist calculate it and store it in Values
            int value = GetValueFor(date);
            Values.Add(new PrecalculateValue{date = date, value = value});

            return value;
        }

        private object GetValueFor(DateTime date)
        {
            //some logic here
        }
    }

我有一个List<PrecalculateValue> Values存储我已计算的所有值供以后使用的地方,我这样做主要是因为我不想为同一个客户端重新计算两次,每次计算涉及大量操作,需要500到1000毫秒,由于孔计费类中涉及一些递归,因此很有可能重用该值.

所有这些都完美地工作,直到我进行测试,我在两个不同的客户端同时进行两次计算,并且该行Values.Single(g => g.date == date).value返回了一个异常,因为它在集合中找到了多个结果.所以我检查了列表,并将两个客户端的值存储在同一个列表中.我该怎么做才能避免这个小问题？

Answer 1

好吧,首先,这一行:

return Values.Single(g => g.date == date).value;

使它永远不会被调用后续行.我猜你在这里稍微解释了你的代码？

如果要将写入同步到Values列表,最直接的方法是lock在要修改列表的代码中的任何位置的公共对象上:

int value = GetValueFor(date);

lock (dedicatedLockObject) {
    Values.Add(new PrecalculateValue{date = date, value = value});
}

return value;

但这里的别的东西值得一提的:因为它看起来像你想有一个PrecalculateValue每DateTime,一个更合适的数据结构很有可能是Dictionary<DateTime, PrecalculateValue>-它会提供快如闪电,O(1)根据您的查找DateTime键,相比于List<PrecalculateValue>这将不得不迭代找到你正在寻找的东西.

有了这个改变,你的代码可能看起来像这样:

public class Billing
{
    private Dictionary<DateTime, PrecalculateValue> Values = 
        new Dictionary<DateTime, PrecalculateValue>();

    private readonly commonLockObject = new object();

    public int GetValue(DateTime date)
    {
        PrecalculateValue cachedCalculation;

        // Note: for true thread safety, you need to lock reads as well as
        // writes, to ensure that a write happening concurrently with a read
        // does not corrupt state.
        lock (commonLockObject) {
            if (Values.TryGetValue(date, out cachedCalculation))
                return cachedCalculation.value;
        }

        int value = GetValueFor(date);

        // Here we need to check if the key exists again, just in case another
        // thread added an item since we last checked.
        // Also be sure to lock ANYWHERE ELSE you're manipulating
        // or reading from the collection.
        lock (commonLockObject) {
            if (!Values.ContainsKey(date))
                Values[date] = new PrecalculateValue{date = date, value = value};
        }

        return value;
    }

    private object GetValueFor(DateTime date)
    {
        //some logic here
    }
}

最后一条建议:除非你的收藏品中存在的特定价值不超过一个是至关重要的,否则该Single方法就会过度.如果您只是想获得第一个值并忽略可能的重复项,First则更安全(例如,更少的异常机会)和更快(因为它不必遍历整个集合).