6

In the boost::shared_ptr destructor, this is done:

if(--*pn == 0)
{
    boost::checked_delete(px);
    delete pn;
}

where pn is a pointer to the reference counter, which is typedefed as

shared_ptr::count_type -> detail::atomic_count -> long

I would have expected the long to be volatile long, given threaded usage and the non-atomic 0-check-and-deletion in the shared_ptr destructor above. Why isn't it volatile?

EDIT:

It turns out I looked at the header used when multi-threaded usage is not specified (atomic_count.hpp). In atomic_count_win32.hpp, the decrement is properly implemented for multithreaded usage.

Johann Gerell
  • 24,991
  • 10
  • 72
  • 122

3 Answers3

16

Because volatile is not necessary for multithreading, and does nothing beneficial, but potentially destroys a number of optimizations.

In order to ensure safe multithreaded access to a variable, the primitive we need is a memory barrier, which provides both the guarantee of volatile and a few others (it prevents memory access reordering across the barrier, which volatile doesn't do)

I believe shared_ptr uses atomic operations when possible, which implicitly provide a memory barrier. Otherwise it falls back to a mutex, which also provides a memory barrier.

See Why is volatile not considered useful in multithreaded C or C++ programming? or http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/ for more details

Edit
count_type is not a long in the general case. It is convertible to a long. If you look in atomic_count.hpp, the typedef to long is only applied if no threading is available (in which case of course, no synchronization is necessary). Otherwise it uses the implementation defined in boost/smart_ptr/detail/atomic_count_pthreads.hpp or boost/smart_ptr/detail/atomic_count_win32.hpp or one of the other files listed. And those are synchronized wrapper classes that ensures all operations are done atomically.

Community
  • 1
  • 1
jalf
  • 243,077
  • 51
  • 345
  • 550
  • "I believe shared_ptr uses atomic operations when possible" - not in my quoted example with the destructor, which is where my problem lies now... – Johann Gerell Mar 26 '10 at 11:24
  • Well, making the variable `volatile` wouldn't have changed anything. You're right, decrementing and comparing a long against zero does seem dangerous, but it depends on what the *rest* of the pointer class is doing. – jalf Mar 26 '10 at 11:35
  • @jalf: Well, that's were you lost me. ;-) Right now, in my example, two threads enter the shared_ptr dtor and "the rest of the pointer class" does nothing else. – Johann Gerell Mar 26 '10 at 11:38
  • @Johann: But again, even if it *is* a bug, how would `volatile` have fixed it? – jalf Mar 26 '10 at 11:40
  • @jalf: Ok, sorry, I wasn't explicit enough - I agree (after reading your links) that volatile is out. But given that fact, I couldn't see how your "it depends on what the rest of the pointer class is doing" could be justified in the dtor example. Note - I'm not saying you're wrong, I just want to understand. – Johann Gerell Mar 26 '10 at 11:47
  • @jalf: Great! My Visual Assist "Goto definition" jumps went nuts and missed that one! atomic_count_win32.hpp perfectly explains my doubts. Thanks! – Johann Gerell Mar 26 '10 at 11:51
  • @jalf: Edited the question to clarify things. – Johann Gerell Mar 26 '10 at 11:56
8

volatile has virtually nothing to do with threading. See here.

Marcelo Cantos
  • 181,030
  • 38
  • 327
  • 365
  • Would you say that there's no problem with the above treatment of the counter in the destructor, if executed on different threads? – Johann Gerell Mar 26 '10 at 11:26
2

You are misreading the code. atomic_count is defined simply as a long if the code is not using multithreading:

#ifndef BOOST_HAS_THREADS

namespace boost
{

namespace detail
{

typedef long atomic_count;

}

}

#elif //... include various platform-specific headers that define atomic_count class

#endif
visitor
  • 8,564
  • 2
  • 26
  • 15