2

I am trying to use std::shared_ptr to point to the data being produced by one thread and consumed by another. The storage field is a shared pointer to the base class,

Here's the simplest Google Test I could create that reproduced the problem:

#include "gtest/gtest.h"
#include <thread>

struct A 
{
  virtual ~A() {}
  virtual bool isSub() { return false; }
};

struct B : public A
{
  bool isSub() override { return true; }
};

TEST (SharedPointerTests, threadedProducerConsumer)
{
  int loopCount = 10000;

  shared_ptr<A> ptr;

  thread producer([loopCount,&ptr]()
  {
    for (int i = 0; i < loopCount; i++)
      ptr = make_shared<B>();              // <--- THREAD
  });

  thread consumer([loopCount,&ptr]()
  {
    for (int i = 0; i < loopCount; i++)
      shared_ptr<A> state = ptr;           // <--- THREAD
  });

  producer.join();
  consumer.join();
}

When run, sometimes gives:

[ RUN      ] SharedPointerTests.threadedProducerConsumer
pure virtual method called
terminate called without an active exception
Aborted (core dumped)

GDB shows the crash with two threads at the locations shown. The stacks follow:

Stack 1

#0  0x00000000006f430a in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffe00008c0)
    at /usr/include/c++/4.8/bits/shared_ptr_base.h:144
#1  0x00000000006f26a7 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7fffdf960bc8, 
    __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:553
#2  0x00000000006f1692 in std::__shared_ptr<A, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7fffdf960bc0, 
    __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:810
#3  0x00000000006f16ca in std::shared_ptr<A>::~shared_ptr (this=0x7fffdf960bc0, __in_chrg=<optimized out>)
    at /usr/include/c++/4.8/bits/shared_ptr.h:93
#4  0x00000000006e7288 in SharedPointerTests_threadedProducerConsumer_Test::__lambda2::operator() (__closure=0xb9c940)
    at /home/drew/dev/SharedPointerTests.hh:54
#5  0x00000000006f01ce in std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda2()>::_M_invoke<>(std::_Index_tuple<>) (this=0xb9c940) at /usr/include/c++/4.8/functional:1732
#6  0x00000000006efe13 in std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda2()>::operator()(void) (
    this=0xb9c940) at /usr/include/c++/4.8/functional:1720
#7  0x00000000006efb7c in std::thread::_Impl<std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda2()> >::_M_run(void) (this=0xb9c928) at /usr/include/c++/4.8/thread:115
#8  0x00007ffff6d19ac0 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#9  0x00007ffff717bf8e in start_thread (arg=0x7fffdf961700) at pthread_create.c:311
#10 0x00007ffff647ee1d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Stack 2

#0  0x0000000000700573 in std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<B, std::allocator<B>, (__gnu_cxx::_Lock_policy)2> > >::_S_destroy<std::_Sp_counted_ptr_inplace<B, std::allocator<B>, (__gnu_cxx::_Lock_policy)2> > (__a=..., __p=0x7fffe00008f0)
    at /usr/include/c++/4.8/bits/alloc_traits.h:281
#1  0x00000000007003b6 in std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<B, std::allocator<B>, (__gnu_cxx::_Lock_policy)2> > >::destroy<std::_Sp_counted_ptr_inplace<B, std::allocator<B>, (__gnu_cxx::_Lock_policy)2> > (__a=..., __p=0x7fffe00008f0)
    at /usr/include/c++/4.8/bits/alloc_traits.h:405
#2  0x00000000006ffe76 in std::_Sp_counted_ptr_inplace<B, std::allocator<B>, (__gnu_cxx::_Lock_policy)2>::_M_destroy (
    this=0x7fffe00008f0) at /usr/include/c++/4.8/bits/shared_ptr_base.h:416
#3  0x00000000006f434c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7fffe00008f0)
    at /usr/include/c++/4.8/bits/shared_ptr_base.h:161
#4  0x00000000006f26a7 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7fffe8161b68, 
    __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:553
#5  0x00000000006f16b0 in std::__shared_ptr<A, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7fffe8161b60, 
    __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:810
#6  0x00000000006f4c3f in std::__shared_ptr<A, (__gnu_cxx::_Lock_policy)2>::operator=<B>(std::__shared_ptr<B, (__gnu_cxx::_Lock_policy)2>&&) (this=0x7fffffffdcb0, __r=<unknown type in /home/drew/dev/unittests, CU 0x0, DIE 0x58b8c>)
    at /usr/include/c++/4.8/bits/shared_ptr_base.h:897
#7  0x00000000006f2d2a in std::shared_ptr<A>::operator=<B>(std::shared_ptr<B>&&) (this=0x7fffffffdcb0, 
    __r=<unknown type in /home/drew/dev/unittests, CU 0x0, DIE 0x55e1c>)
    at /usr/include/c++/4.8/bits/shared_ptr.h:299
#8  0x00000000006e7232 in SharedPointerTests_threadedProducerConsumer_Test::__lambda1::operator() (__closure=0xb9c7a0)
    at /home/drew/dev/SharedPointerTests.hh:48
#9  0x00000000006f022c in std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda1()>::_M_invoke<>(std::_Index_tuple<>) (this=0xb9c7a0) at /usr/include/c++/4.8/functional:1732
#10 0x00000000006efe31 in std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda1()>::operator()(void) (
    this=0xb9c7a0) at /usr/include/c++/4.8/functional:1720
#11 0x00000000006efb9a in std::thread::_Impl<std::_Bind_simple<SharedPointerTests_threadedProducerConsumer_Test::TestBody()::__lambda1()> >::_M_run(void) (this=0xb9c788) at /usr/include/c++/4.8/thread:115
#12 0x00007ffff6d19ac0 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#13 0x00007ffff717bf8e in start_thread (arg=0x7fffe8162700) at pthread_create.c:311
#14 0x00007ffff647ee1d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

I have tried various approaches here, including using std::dynamic_pointer_cast but I haven't had any luck.

In reality the producer stores many different subclasses of A by their type_id in a std::map<type_id const*,std::shared_ptr<A>> (one instance per type) which I look up from the consumer by type.

My understanding is that std::shared_ptr is threadsafe for these types of operations. What am I missing?

Drew Noakes
  • 300,895
  • 165
  • 679
  • 742
  • access one variable in two threads without mutex - this is what is wrong – BЈовић May 17 '13 at 20:30
  • 4
    No, these operations are not _thread-safe_. Take a look on atomic operations for shared_ptr: http://en.cppreference.com/w/cpp/memory/shared_ptr/atomic – nosid May 17 '13 at 20:34
  • @nosid, does that make [this statement on Wikipedia](http://en.wikipedia.org/wiki/Smart_pointer#Concurrency_guarantees) incorrect? It suggests a behaviour I did not observe, at least not from g++. – Drew Noakes May 20 '13 at 09:33
  • @DrewNoakes: I don't understand your comment. The URL refers to three statements: 1. Individual shared_ptr objects require proper synchronization. That's the reason why your code is not working. 2. Except for the atomic operations. That's the link I have posted above. 3. Multiple threads can access different shared_ptr objects without synchronization. That's an important guarantee but does not apply to your code. – nosid May 20 '13 at 09:58
  • @DrewNoakes: it is the *writing* that causes problems, not the accessing. – Klaas van Gend Mar 07 '14 at 15:10

1 Answers1

3

shared_ptr has thread-safety on its control block. When a shared_ptr is created and points to a newly created resource it creates a control block. According to MSDN this holds:

The shared_ptr objects that own a resource share a control block. The control block holds:

  • the number of shared_ptr objects that own the resource,
  • the number of weak_ptr objects that point to the resource,
  • the deleter for that resource if it has one,
  • the custom allocator for the control block if it has one.

This means that shared_ptr will ensure that there are no synchronization issues with multiple copies of shared_ptr pointing to the same memory. However, it does not manage the synchronization of the memory itself. See the section on thread safety (emphasis mine)

Multiple threads can read and write different shared_ptr objects at the same time, even when the objects are copies that share ownership.

Your code shares ptr which means you have a data race. Also note that it is possible for your producer thread to produce several objects before the consumer thread is scheduled to run, meaning that you lose some objects.

As has been pointed out in a comment, you can use atomic operations on shared_ptr. The producer thread then looks like:

thread producer([loopCount,&ptr]()
{
    for (int i = 0; i < loopCount; i++)
    {
        auto p = std::make_shared<B>();  // <--- THREAD
        std::atomic_store<A>( &ptr, p );
    }          
});

The object is created and then atomically stored into ptr. The consumer then needs to atomically load the object.

thread consumer([loopCount,&ptr]()
{
    for (int i = 0; i < loopCount; i++)
    {
        auto state = std::atomic_load<A>( &ptr );           // <--- THREAD
    }
});

This still has the disadvantage that objects will be lost when the producer thread is allowed to run for multiple iterations.

These examples were written in Visual Studio 2012. At this time, gcc hasn't fully implemented atomic shared_ptr access, as noted in section 20.7.2.5

Steve
  • 7,171
  • 2
  • 30
  • 52
  • Thanks for the example of using the atomic operations. I don't care about skipping objects -- the producer is sensing data every 8ms from a set of motors and the consumer is sampling from that data every 30ms. The consumer should always have the latest available data. – Drew Noakes May 17 '13 at 21:56
  • Your code examples [don't compile](http://pastebin.com/w3Nyu3iq) for me using g++ 4.8.0. Is this specific to VC++? – Drew Noakes May 17 '13 at 22:06
  • Yes, I wrote this in VC++, however, it shouldn't be specific to VC++. Looking at [C++11 std lib page](http://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.200x), section 20.7.2.5 it looks like gcc only has partial support at present. Sorry, I didn't realise when answering. – Steve May 17 '13 at 22:15