Are smart pointers threadsafe?
I'm using the Boost smart pointers but I have a feeling they're not threadsafe? Intrusive_ptr could quite easily be made threadsafe because by design you control the actual counter and accesses to it. Shared_ptr is harder because it would involve modifying the source code.
Now that smart pointers are becoming part of the C++ standard I think they should be considered for TBB support (just like TBB has its own thread-safe container implementations). I realize not everything can be included but smart pointers are widely used and notoriously hard to implement so they're good candidates.
What do you think?
| |
Re: Are smart pointers threadsafe?
 Raf_Schietekat:
The reference says shared_ptr offers the same thread-safety as built-in types. I've interpreted this as they're like integers for example which need to be updated in atomic operations if they're shared among threads and simultaneously read and written. This is why TBB offers atomic<T> isn't it?
Well, I may be wrong but I don't think shared_ptr is "atomic". Could someone please shed some light on this.
| |
Re: Are smart pointers threadsafe?
The documentation could be made clearer, but shared_ptr is thread-safe with regard to its reference counter (which is all you really need), even if the shared_ptr object itself is not thread-safe.
Once you have safely copied or assigned p to p1 and p2, e.g., as automatic variables or parameters, which therefore both refer to the same object underneath (the referent), you can do whatever you like to p1 in thread 1 and whatever you like to p2 in thread 2: dereference (as long as the referent is thread-safe for those accesses), reassign, reset, make further copies, anything. If you access the identical pointer instance however, e.g., as a member variable in a shared object, you need a mutex if any of those accesses are writes, perhaps a read/write mutex; there would probably be little or no benefit in extending thread safety to shared_ptr instances, which would become bigger and slower as a result, although you could easily encapsulate a shared_ptr if you would like to do that anyway.
I just don't understand example 4 (how can p2 be used in thread A and go out of scope in thread B?), and they probably should have added some examples of simultaneous use of separate instances.
(Added) I don't think a competing smart pointer should be added to TBB: boost has intentionally limited its number of possible smart pointers as much as it could for the sake of interoperability (however tempting it might be to add more with special behaviour), which is a very good reason.
| |
Re: Are smart pointers threadsafe?
I have used boost::shared_ptr in TBB applications before and it seemed to work just fine. I believe there are atomic increment and decrement operations within it. For some reason I think there is a mutex buried in there as well, but I'm not sure that there is... I would ask on the Boost mailing lists for a better answer... it should be possible to replace the boost atomic operations with ones from TBB, again ask on the boost mailing list for more details.
In my experience, boost::shared_ptr had horrible performance. However, this is all relative. In my case, the pointers would increment and decrement millions of times in a short time... the majority of my algorithm related to pointer manipulations, so it became a bottleneck in that case... which looking back on it I say "of course". If you have a large number of pointers, which are being used a lot... I would consider a different approach, but of course that was just based on my experience, and code profiling revealed that something insane like 60% of execution time was spent on shared_ptr atomic operations.
I have written pointer containers that use TBB constructs instead, you might be interested in those. They are used for exception safety, to call delete on all contained pointers if it goes out of scope. Boost has something similar too.
| |
Re: Are smart pointers threadsafe?
Thank you both. I feel confident now that the Boost shared_ptr is as thread-safe as I need it to be.
With Boost shared_ptr as thread-safe as they are it shouldn't be necessary to have them in TBB too, unless there is some significant advantage like they could be made much more efficient or even safer or something.
Regarding the efficiency of reference counting per se. I'm aware that this kind of memory management comes at a price so I'm careful about not overusing it and I don't use it in "algorithm style" code.
| |
Re: Are smart pointers threadsafe?
Architecture-specific reference count management underneath shared_ptr is in boost/detail/sp_counted_base*.hpp. Note that it uses gcc where TBB confusingly uses linux (in tbb/machine). From a quick glance at gcc_x86 it seems decent enough?
(Added) A mutex is used in boost if there is no architecture-specific support for atomic access to a variable, at the cost of increasing its size and lowering its performance. TBB atomics are the size of the underlying type and always require a minimal set of supported architecture-specific operations, so it will not (be able to) fall back to using mutexes.
| |
Re: Are smart pointers threadsafe?
I know now that Shared_ptr is thread-safe enougth for my needs and often the best solution but sometimes one wants a more lightweight alternative and I've been using Intrusive_ptr. To make it easier to use I've stolen this class from Beyond the C++ Standard Library by B. Karlsson, If a class inherits it publicly it becomes "intrusive enabled". class IntrusiveCount { // base class to enable derived class for use with the Boost intrusive smart pointer public: virtual ~IntrusiveCount(){} friend void intrusive_ptr_add_ref(IntrusiveCount* p) { ++p->ref_count; } friend void intrusive_ptr_release(IntrusiveCount* p) { if (--p->ref_count == 0) delete p; } protected: IntrusiveCount() : ref_count(0) {} IntrusiveCount& operator=(const IntrusiveCount&) { return *this; } private: IntrusiveCount(const IntrusiveCount&); short ref_count; };
The problem with the above class is that its not thread-safe so I've tried to modified it using TBB. I would appreciate if someone could have a look at it and tell me if there's something wrong with my approach. class IntrusiveCount { // thread-safe base class to enable derived class for use with the Boost intrusive smart pointer public: virtual ~IntrusiveCount(){} friend void intrusive_ptr_add_ref(IntrusiveCount* p) { p->ref_count.fetch_and_add(1); // increment counter atomically } friend void intrusive_ptr_release(IntrusiveCount* p) { if (p->ref_count.fetch_and_add(-1) == 1) delete p; // decrement counter. If it was 1 before it's 0 now so delete } protected: IntrusiveCount() {ref_count=0;} IntrusiveCount& operator=(const IntrusiveCount&) { return *this; } private: IntrusiveCount(const IntrusiveCount&); tbb::atomic<int> ref_count; }; I had to change the counter to an int (from short) otherwise I get this level 2 warning on VS2008 standard, C:UsersadminDocumentsMint bbsrcinclude bbatomic.h(157) : warning C4244: 'argument' : conversion from 'tbb::internal::atomic_traits<Size,M>::word' to 'short', possible loss of data
| |
Re: Are smart pointers threadsafe?
Just as an FYI, this is what I use. The code is available via SVN, and you are free to use it.
There are some improvements that can be made to this code, but here it is.
/*
Copyright 2007-2008 Adrien Guillon. All Rights Reserved.
This file is part of TBB Community Code.
TBB Community Code is free software; you can redistribute it
and/or modify it under the terms of the GNU General Public License
version 2 as published by the Free Software Foundation.
TBB Community Code is distributed in the hope that it will be
useful, but WITHOUT ANY WARRANTY; without even the implied warranty
of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with TBB Community Code; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
As a special exception, you may use this file as part of a free software
library without restriction. Specifically, if other files instantiate
templates or use macros or inline functions from this file, or you compile
this file and link it with other files to produce an executable, this
file does not by itself cause the resulting executable to be covered by
the GNU General Public License. This exception does not however
invalidate any other reasons why the executable file might be covered by
the GNU General Public License.
*/
#ifndef __TCC_ptr_vector_H
#define __TCC_ptr_vector_H
#include <algorithm>
#include <vector>
#include <tbb/spin_mutex.h>
#include <boost/checked_delete.hpp>
namespace tcc {
/**
* A vector that stores pointers to "owned objects." This container assumes
* ownership of the pointers it holds, and will call delete on them when this
* object is destructed, or appopriate member function calls are made.
*
* The goal of this class is to provide a method of storing objects in an
* exception safe manner, without using smart pointers.
*
* Instantiate the template with the type of object it will contain, not
* with pointers itself.
*
* The Mutex can be any TBB mutex type, including RW types.
*
* The deleter template argument is for a function object which
* implements void operator()(T* x), and deletes the object. By
* default, normal C++ delete is called.
*/
template <typename T, typename Container = typename std::vector<T*>, typename Mutex = tbb::spin_mutex, typename Deleter = boost::checked_deleter<T> >
class ptr_vector
{
public:
typedef Container container_type;
typedef Mutex mutex_type;
typedef typename Container::iterator iterator;
; typedef T* element_type;
/**
* Construct an empty ptr_vector
*/
ptr_vector() { }
/**
* The destructor calls delete on all contained object pointers.
*
* TODO: Should the destructor use the mutex?
*/
~ptr_vector()
{
if(!empty())
{
deleteAll();
}
}
/**
* Takes ownership of an object. This container will now own the object,
* and is responsible for the destruction of the object unless the user
* destroys it themselves with delete_member(x) or steal_member(x).
*
* This function is thread safe.
*/
T& giveOwnership(T* x)
{
{ // BEGIN MUTEX
typename Mutex::scoped_lock lock(_ownedObjectsMutex);
_ownedObjects.push_back(x);
} // END MUTEX
return *x;
}
/**
* Take ownership of an object away from this container. The object
* will no longer be owned by this container, and a pointer will be
* returned to the user.
*
* This function is thread safe.
*/
T* takeOwnership(T& x)
{
typename container_type::iterator i;
T* objectToRemove;
{ // BEGIN MUTEX
typename Mutex::scoped_lock lock(_ownedObjectsMutex);
i = std::remove(_ownedObjects.begin(), _ownedObjects.end(), &x);
objectToRemove = *i;
_ownedObjects.erase( i, _ownedObjects.end() );
} // END MUTEX
return objectToRemove;
}
/**
* Call delete on an object owned by this container. The object deleted
* is removed from ownership.
*
* This function is thread safe.
*/
void deleteMember(T& x)
{
&nbs
p; T* objectToDelete;
objectToDelete = takeOwnership(x);
_deleter(objectToDelete);
}
/**
* Call delete on all objects owned by this container. All objects
* deleted are removed from ownership. A mutex is used to govern
* access of internal data structures. This is the same function which
* is called by the destructor.
*
* This function is thread safe.
*/
void deleteAll()
{
{ // BEGIN MUTEX
typename Mutex::scoped_lock lock(_ownedObjectsMutex);
std::for_each(_ownedObjects.begin(), _ownedObjects.end(), _deleter);
_ownedObjects.clear();
} // END MUTEX
}
/**
* Check if this container owns anything.
*
* This function is thread safe.
*
* @return true if there are no objects owned, false otherwise.
*/
bool empty() const
{
return _ownedObjects.empty();
}
/**
* Swaps the contents of this ptr_vector with a ptr_vector instance
* provided as an argument.
*
* This function is thread safe, however it requires two mutexes,
* one for this ptr_vector and one for the ptr_vector argument.
*/
void swap(ptr_vector& x)
{
{ // BEGIN MUTEX ON x
typename Mutex::scoped_lock lock_x(x._ownedObjectsMutex);
{ // BEGIN MUTEX ON this
typename Mutex::scoped_lock lock_this(_ownedObjectsMutex);
_ownedObjects.swap(x);
}
}
}
iterator begin() { return _ownedObjects.begin(); }
iterator end() { return _ownedObjects.end(); }
private:
/**
* Actual object container.
*/
Container _ownedObjects;
/**
* Copying a ptr_vector is forbidden in case the user accidently
&
nbsp; * does so. If copying a ptr_vector were permitted, the contents
* of the rvalue would have to be cleared, which might not be
* what the user expected. Instead, the user must call the function
* swap().
*/
ptr_vector(const ptr_vector& x);
/**
* This is illegal so that users can't accidently lose memory. They
* should instead use swap().
*/
ptr_vector& operator=(const ptr_vector&);
/**
* Instantiate an actual instance of the deleter.
*/
Deleter _deleter;
/**
* Mutex for the internal data structure.
*/
Mutex _ownedObjectsMutex;
};
template <typename T>
class ptr_vector_default : public ptr_vector<T, std::vector<T*>, tbb::spin_mutex, boost::checked_deleter<T> >
{
};
} // END NAMESPACE tcc
#endif
| |
Re: Are smart pointers threadsafe?
btw, feel free to come onto #tbb on irc.freenode.net, I'd love to help in real-time.
AJ
| |
Re: Are smart pointers threadsafe?
uj: I would appreciate if someone could have a look at it and tell me if there's something wrong with my approach. class IntrusiveCount { // thread-safe base class to enable derived class for use with the Boost intrusive smart pointer
public:
...//skipped when quoting
friend void intrusive_ptr_release(IntrusiveCount* p) { if (p->ref_count.fetch_and_add(-1) == 1) delete p; // decrement counter. If it was 1 before it's 0 now so delete } ...//skipped when quoting
private:
IntrusiveCount(const IntrusiveCount&);
tbb::atomic<int> ref_count;
};
I had to change the counter to an int (from short) otherwise I get this level 2 warning on VS2008 standard, C:UsersadminDocumentsMint bbsrcinclude bbatomic.h(157) : warning C4244: 'argument' :
conversion from 'tbb::internal::atomic_traits<Size,M>::word' to 'short', possible loss of data
You did it almost right, except for one thing. tbb::atomic<>::fetch_and_add returns the value before the atomic addition, i.e. it is equivalent to a postfix increment, while the original code used a prefix increment.
In fact, you do not need to use fetch_and_add at all, because tbb::atomic<> overloads increment and decrement operators, in both postix and prefix forms. So the only change in the original code would be in the declaration of ref_count as tbb::atomic<int>.
By the way, which TBB package did you use? I wonder about this VS2008 warning. I believe our tests would show it; so most probably, it has been fixed in latest updates of TBB.
| | |
