提问者:小点点

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


下面是在动作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){...}。我说的对吗?


共1个答案

匿名用户

>

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

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

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

  • 相关问题