How to use boost::atomic_store with shared_ptr and shared_ptr?

Question

In my case T is pcl::PointCloud<pcl::PointXYZ>> but the question shoud stand for any type T. The following example produces an error:

using pc = pcl::PointCloud<pcl::PointXYZ> >;
boost::shared_ptr<pc> p(new pc);
boost::shared_ptr<const pc> const_p(new pc);

// This is legal
const_p = p;

// The atomic equivalent is not
boost::atomic_store(&const_p, p);

The problem is that the boost::atomic_store expects both arguments to be T* and T, but these are considered different types despite the fact that it's perfectly safe to assign p to const_p. The following doesn't work either.

boost::atomic_store(&const_p, const_cast<boost::shared_ptr<const pc> > (p));

Despite the above basically casting a pc* to const pc* which is perfectly safe, it produces an error about const_cast not being able to convert to different type. I understand that because pc is a template argument, it is considered part of the type of the shared_ptr and not a cv qualification. The following work

boost::atomic_store(&const_p, boost::shared_ptr<const pc>(p));

However, it creates an extra unecessary boost::shared_ptr. It is my understanding that the same is true for boost::const_pointer_cast<const pc>(p) This can be avoided if p is no longer needed.

boost::atomic_store(&const_p, boost::shared_ptr<const pc>(std::move(p));

This still creates an extra object but it shouldn't matter because the reference count is not modified, which is the expensive part of copying a shared_ptr on account of being atomic.

It just so happens that this occurs in a non-critical part of my code so I'm fine with the above, but I would like to know for future reference: If std::move was not an option, how would one atomically store a boost::shared_ptr<T> to a boost::shared_ptr<const T> without the overhead of creating an unecessary temporary pointer? It should be possible because it is safe to view a T through a const T*, but I can't figure out a way to do it.

score 2 · Accepted Answer · answered Sep 11 '17 at 14:05

I understand that because pc is a template argument, it is considered part of the type of the shared_ptr and not a cv qualification.

Yes, this is known as "non-deducible context".

The following work
boost::atomic_store(&const_p, boost::shared_ptr<const pc>(p));
However, it creates an extra unecessary boost::shared_ptr. It is my understanding that the same is true for boost::const_pointer_cast<const pc>(p) This can be avoided if p is no longer needed.

Well, surprise, you always get the copy:

template<class T> void atomic_store( shared_ptr<T> * p, shared_ptr<T> r ) BOOST_SP_NOEXCEPT
{
    boost::detail::spinlock_pool<2>::scoped_lock lock( p );
    p->swap( r );
}

Note the second parameter is by value. This, at once, solves the mystery:

Live On Coliru

#include <boost/shared_ptr.hpp> 
#include <boost/make_shared.hpp> 
#include <boost/atomic.hpp>

namespace pcl {
    struct PointXYZ {};
    template <typename P> struct PointCloud {
    };
}

int main() {
    using pc = pcl::PointCloud<pcl::PointXYZ>;
    boost::shared_ptr<pc> p             = boost::make_shared<pc>();
    boost::shared_ptr<const pc> const_p = boost::make_shared<pc>();

    // This is legal
    const_p = p;

    // The atomic equivalent is too
    boost::atomic_store<pc const>(&const_p, p);
}

If std::move was not an option, how would one atomically store a boost::shared_ptr to a boost::shared_ptr without the overhead of creating an unecessary temporary pointer?

You can't. Look at it this way: load/store are meant to be trivial operations amenable to atomic lockfree implementations. They do 1 thing, and they do it well¹.

Doing implicit conversions is just not the responsibility of that function.

I'd suggest using a wrapper function, or even using ADL to resolve your own overload from your own namespace.

I don't know what I was thinking. Since the template takes `r` by value anyway, it will always require at least a copy or a move, and conversion to const will add a move but not an extra copy. Is there a reason these templates are not designed to take `r` by reference? Is it for thread-safety? — patatahooligan, Sep 11 '17 at 14:15
Good question. I just checked and the C++11 specs say the same. http://en.cppreference.com/w/cpp/memory/shared_ptr/atomic I'll ask for assistance in [the lounge](https://chat.stackoverflow.com/rooms/10/loungec) — sehe, Sep 11 '17 at 14:17
A copying a `shared_ptr` is a deref and an atomic increment or two. Also if `r` was a reference then the user code may have `p` and `r` be the exact same `shared_ptr` and I don't know what the consequences would be for that. Doing copy-swap is simpler and safer. — ratchet freak, Sep 11 '17 at 14:33
There's also a matter of C compatibility. Quite a bit of effort was put into keeping C++11 and C11 atomic operations compatible. Among other things that means they avoid references (almost?) entirely. Where a function needs to modify something, it's passed a pointer. Otherwise, it uses a value. — Jerry Coffin, Sep 11 '17 at 15:18
It took me some time to get it, but the copy and swap doesn't really add much overhead. Swap is non-atomic because it only affects the pointers to the object and the control block. The copy is necessary anyway because it needs to be stored in `*p`. The lock would be required either way because it's an atomic store. And the conversion to const like I said earlier will add a single move because the `shared_ptr` will be a temporary. So I think this answer with its comments has covered the question. — patatahooligan, Sep 21 '17 at 13:18

How to use boost::atomic_store with shared_ptr and shared_ptr?

1 Answers1

Linked