这个使用锁的线程安全列表是完美的吗？

Question

下面是在动作2中使用来自C++并发的锁的线程安全列表的示例源代码。

template<typename T>
class threadsafe_list
{
    struct node
    {
        std::mutex m;
        std::shared_ptr<T> data;
        std::unique_ptr<node> next;
        node():
            next()
        {}
        node(T const& value):
            data(std::make_shared<T>(value))
        {}
    };
    node head;
public:
    threadsafe_list()
    {}
    ~threadsafe_list()
    {
        remove_if([](node const&){return true;}); // (1) (2)
    }
    threadsafe_list(threadsafe_list const& other)=delete;
    threadsafe_list& operator=(threadsafe_list const& other)=delete;
    void push_front(T const& value)
    {
        std::unique_ptr<node> new_node(new node(value));
        std::lock_guard<std::mutex> lk(head.m);
        new_node->next=std::move(head.next);
        head.next=std::move(new_node);
    }
    template<typename Function>
    void for_each(Function f)
    {
        node* current=&head;
        std::unique_lock<std::mutex> lk(head.m);
        while(node* const next=current->next.get())
        {
            std::unique_lock<std::mutex> next_lk(next->m);
            lk.unlock();
            f(*next->data);
            current=next;
            lk=std::move(next_lk);
        }
    }
    template<typename Predicate>
    std::shared_ptr<T> find_first_if(Predicate p) // (3)
    {
        node* current=&head;
        std::unique_lock<std::mutex> lk(head.m);
        while(node* const next=current->next.get())
        {
            std::unique_lock<std::mutex> next_lk(next->m);
            lk.unlock();
            if(p(*next->data))
            {
                return next->data;
            }
            current=next;
            lk=std::move(next_lk);
        }
        return std::shared_ptr<T>();
    }
    template<typename Predicate>
    void remove_if(Predicate p)
    {
        node* current=&head;
        std::unique_lock<std::mutex> lk(head.m);
        while(node* const next=current->next.get())
        {
            std::unique_lock<std::mutex> next_lk(next->m);
            if(p(*next->data)) // (2) - 1
            {
                std::unique_ptr<node> old_next=std::move(current->next);
                current->next=std::move(next->next);
                next_lk.unlock();
            }
            else
            {
                lk.unlock();
                current=next;
                lk=std::move(next_lk);
            }
        }
    }
};

我理解这段代码的工作原理。但我不认为这个代码是完美的。我在这三个地方做了记号。

（1）析构函数中的remove_if确有必要？节点中的每个数据都使用智能指针。所以我不认为析构函数必须移除列表中的元素。你的意见呢？

（2）即使使用remove_if,lambda函数的参数[]（节点const&）{...}看起来怪怪的。我认为lambda函数应该是[](T const&){...}这样的函数。这是因为如果使用节点类型，在点(2)-2处，它使用*next->data作为参数，所以从T到node的隐式类型转换发生在node构造函数中，这是冗余。如果我们使用T类型，就不必进行隐式转换。

(3)find_first_if返回shared_ptr而不是复制的T值，这给并发问题提供了机会。我会解释更多。如果用户使用find_first_if获得数据的shared_ptr,那么即使在修改节点时也可以通过该指针访问数据。这是安全的行动吗？我不这么认为。我的建议是，它应该返回T的复制值，这将导致T find_first_if（谓词p）{...}。我说的对吗？

Answer 1

>

这实际上是必要的，不是为了防止泄漏，而是为了防止堆栈溢出。由于节点彼此拥有，因此每个节点的析构函数将递归调用以下所有节点的析构函数。因此破坏一个长列表将很容易炸毁堆栈。

是的，这是不正确的，您应该查阅谓词的文档。更好的方法是使用C++14[](const auto&){return true；}，但由于缺少make_unique，代码可能只使用C++11编写。

是的，公开的数据不再受任何锁的保护。复制可能会很贵，而仅移动的类型呢？同样，这应该在方法中适当地记录下来。