什么时候再锁定再锁定重构是个坏主意？

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() 的调用都是序列化的；互斥锁用于其他使用访问函数的线程。

问。将一个互斥锁拆分为两个串行完成的锁不会增加任何新的死锁可能性？

问。与 A 类和 B 类的代码重复，或“隐藏”基类调用中的额外互斥锁是否会产生更大的代码异味？

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

问。 class C 现在允许读取 values 与读取 history 不同步（即，如果另一个线程在 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...
 }

};