这种锁定然后再次锁定的重构什么时候是一个坏主意

Question

我有两个同级类，A和B，我想重构它们，以便A是B的父级，这样B就可以共享A的代码。但这种重构意味着，对于一个关键函数，锁定互斥锁两次而不是一次。有什么理由不这样做？

A类

class A{
     std::map<std::string, int> values;
     std::mutex mutex;
 public:
     //init and access functions elided
     void foo(std::string key, int v){
         auto i = values.find(key);
         if(i == values.end())return; //Actually report error
         {
             std::lock_guard<std::mutex> lock(mutex);
             i->second = v;
         }
     }
};

B类

class B{
    std::map<std::string, int> values;
    std::map<std::string, std::vector<int> > history;
    std::mutex mutex;
public:
    //init and access functions elided
    void foo(std::string key, int v){
        auto i = values.find(key);
        if(i == values.end())return; //Actually report error
        auto i2 = history.find(key);
        if(i2 == history.end())return; //Actually report error
        {
            std::lock_guard<std::mutex> lock(mutex);
            i->second = v;
            i2->second.push_back(v);
        }
    }
 };

我想写的B类的替代方案是：

class C:public A{
    std::map<std::string, std::vector<int> > history;
public:
    //init and access functions elided
    void foo(std::string key, int v){
        A::foo(key,v);
        auto i2 = history.find(key);
        if(i2 == history.end())return; //Actually report error
        {
            std::lock_guard<std::mutex> lock(mutex);
            i2->second.push_back(v);
        }
    }
 };

访问函数还使用互斥锁来锁定其读取。出于此问题的目的，假设foo()被调用的次数比这些类中的任何其他函数都要多得多。我们还可以假设对foo()的所有调用都是序列化的;互斥锁适用于使用 Access 函数的其他线程。

Q. 将一个互斥锁拆分为两个串行锁不会增加任何新的死锁潜力？

问。与类 A 和类 B 的代码重复，还是在基类调用中"隐藏"额外的互斥锁，是否有更大的代码气味？

问。与我在foo()中执行的其他操作相比，锁定两次的额外开销是否微不足道？即我猜插入到地图和矢量中所需的时间至少是锁定互斥锁的 10 倍。

问。 class C现在允许读取与读取history不同步的values(即，如果另一个线程在C::foo()中间抓住了锁(。如果事实证明这是一个问题，那么回到"复制A类和B类中的代码"是唯一的设计选择吗？

Answer 1

这个替代方案怎么样，它添加一个返回锁的foo_impl函数，因此可以在C::foo中重复使用：

class A
{
  std::map<std::string, int> values;
  std::mutex mutex;
public:
  //init and access functions elided
  void foo(std::string key, int v)
  {
    foo_impl(key, v);
  }
protected:
  std::unique_lock<std::mutex> foo_impl(std::string key, int v)
  {
    auto i = values.find(key);
    if (i == values.end()) return {}; //Actually report error
    std::unique_lock<std::mutex> lock(mutex);
    i->second = v;
    return lock;
  }
};
class C : public A
{
  std::map<std::string, std::vector<int> > history;
public:
  //init and access functions elided
  void foo(std::string key, int v)
  {
    auto i2 = history.find(key);
    if (i2 == history.end()) return; //Actually report error
    if (auto lock = A::foo_impl(key,v))
      i2->second.push_back(v);
  }
};

这可确保对A::values和C::history的更新在单个锁下完成，因此A::values无法在原始C::foo中的两个锁之间再次更新。

Answer 2

我不

明白为什么你认为有必要锁定两次，这似乎是你想做的？

class A{
  std::map<std::string, int> values;
  std::mutex mutex;
protected:
  void foo_unlocked(std::string key, int v){
    auto i = values.find(key);
    if(i != values.end())
      i->second = v;
  }
};
class C:public A{
  std::map<std::string, std::vector<int> > history;
public:
  //init and access functions elided
  void foo(std::string key, int v){
    auto i2 = history.find(key);
    if(i2 == history.end())
      return; //Actually report error
    std::lock_guard<std::mutex> lock(mutex);
    i2->second.push_back(v);
    foo_unlocked(key, v);  // do the operation in A but unlocked...
 }

};