Strange shared_ptr behaviour
Asked Answered
E

1

5

Ive discovered a really strange behaviour with std::shared_ptr in c++. The following example works perfectly with standard pointers.

However the usage of std::shared_ptr causes here a segmentation fault. (see the backtrace below)

I know, that accessing a std::shared_ptr from multiple threads is not safe, therefor I'm using the atomic-operations. Even a classic lock wont solve the problem.

Im using gcc version 6.3.0 20170406 (Ubuntu 6.3.0-12ubuntu2) with -Wall -O2 -g and -std=c++17

Does anybody know a solution or why the code is crashing?

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>

template <typename T>
class list {
private:
    struct Node {
        T value;
        std::shared_ptr<Node> next;

        Node() : next(nullptr) {}
    };

    std::shared_ptr<Node> head;
public:
    // create dummy-nodes
    explicit list() : head(std::make_shared<Node>()){
        head->next = std::make_shared<Node>();
    };

    void push_front(T val) {
        std::shared_ptr<Node> current;
        std::shared_ptr<Node> next;
        std::shared_ptr<Node> newNode = std::make_shared<Node>();
        newNode->value = val;

        do {
            current = std::atomic_load<Node>(&head);
            next = std::atomic_load<Node>(&current->next);
            newNode->next = next;
        } while (!std::atomic_compare_exchange_weak<Node>(&current->next, &next, newNode));
    }
};

int main(int argc, char* argv[]) {

    list<int> ll;

    std::vector<std::thread> threads;
    const int thread_count = 8;
    const int local_count = 200000;

    std::mutex m;

    for (int i = 0; i < thread_count; i++) {
        threads.push_back(std::thread([&ll, i, local_count, &m]() {
            for (int j = local_count * i; j < local_count * (i + 1); j++) {
                m.lock(); // optional for testing; doesnt solve the problem
                ll.push_front(j);
                m.unlock();
            }

        }));
    }

    for (auto& thrd : threads) {
        thrd.join();
    }

    return 0;
}

gdb-backtrace:

0x0000555555555d4e in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb0e0) at /usr/include/c++/6/bits/shared_ptr_base.h:150
150         _M_dispose();
(gdb) bt
#0  0x0000555555555d4e in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb0e0) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#1  std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#2  std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#3  std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#4  list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#5  __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#6  std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#7  std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#8  0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb110) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#9  std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#10 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#11 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#12 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#13 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#14 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#15 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#16 0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb140) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#17 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#18 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#19 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#20 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#21 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#22 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#23 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#24 0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb170) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#25 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#26 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#27 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#28 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#29 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#30 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#31 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#32 0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb1a0) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#33 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#34 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#35 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#36 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#37 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#38 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#39 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#40 0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb1d0) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#41 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#42 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#43 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#44 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#45 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#46 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#47 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
#48 0x0000555555555d51 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffd48eb200) at /usr/include/c++/6/bits/shared_ptr_base.h:150
#49 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:662
#50 std::__shared_ptr<list<int>::Node, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:928
#51 std::shared_ptr<list<int>::Node>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/6/bits/shared_ptr.h:93
#52 list<int>::Node::~Node (this=<optimized out>, __in_chrg=<optimized out>) at src/main.cpp:11
#53 __gnu_cxx::new_allocator<list<int>::Node>::destroy<list<int>::Node> (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/6/ext/new_allocator.h:124
#54 std::allocator_traits<std::allocator<list<int>::Node> >::destroy<list<int>::Node> (__a=..., __p=<optimized out>) at /usr/include/c++/6/bits/alloc_traits.h:487
#55 std::_Sp_counted_ptr_inplace<list<int>::Node, std::allocator<list<int>::Node>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=<optimized out>) at /usr/include/c++/6/bits/shared_ptr_base.h:529
Errand answered 24/5, 2017 at 14:4 Comment(5)
@clcto Take a look at this.Reference
Are you sure you need shared_ptr instead of unique_ptr? I don't see how you can have multiple owners of linked list nodes.Reference
you cannot access the value of a linked-Node in a lockfree list. Hence you need a local reference to the object to prevent the deallocation after removing the Node.Errand
Please note that, at least in your example, you do not need std::atomic_load since you are using a mutex that synchronize list access, anyway the problem you have is dued to a stack overflow (not this site!) in the recursive destructor. As another comment says if you need counted pointers try to use std::unique_ptr, it fits better this use case, or use a std::list of std::shared_ptr (that does not recurse in destructor).Mountford
the mutex was just for debugging to prevent any data-races in push_front. shared_ptr are actually the only option in a lockfree-list, where multiple threads can access the same Node simultaneouslyErrand
Z
9

This is a classic problem with smart pointers and list nodes: the destructor of your node is recursive and it ends up running out of stack. This is what that stacktrace shows: stack overflow.

In CppCon 2016: Herb Sutter “Leak-Freedom in C++... By Default.” they discuss exactly this problem.

Zanazander answered 24/5, 2017 at 14:12 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.