Peter Dimov wrote:
John Torjo wrote:
Peter Dimov wrote:
John Torjo wrote:
Hi Peter,
I think there is a bug in your Interlocked* implementation of shared_ptr (http://www.pdimov.com/cpp/shared_count_x86_exp2.hpp).
In atomic_read you have: inline long atomic_read(long volatile const & value) { return value; }
I don't believe this is thread-safe.
(repost)
On x86/IA32, I believe that it's as thread safe as you can get. Note that all count updates go through Interlocked* calls. On IA32, plain reads can only return a somewhat "surprising" value when interleaved with plain writes, and even then, the element of surprise is limited. ;-)
I kind of wonder.
Here's my test scenario, which I think is quite effective ;)
The main thread creates a window. Then it internally has a shared pointer to it. Other threads might want access to this window. Each other thread keeps a weak_pointer to it.
Whenever a thread (other than main) wants to access this window, it will query the weak_pointer. The weak_pointer needs to know the LATEST reference count in order to know if the weak pointer is still valid. Thus, atomic_read that simply returns the value is not enough.
atomic_read is only called by use_count. You shouldn't be using use_count to check whether a weak_ptr is still valid, because this can never be thread safe, regardless of the implementation of atomic_read. Use weak_ptr::lock.
Lets take a look at weak_ptr::lock(): wnd_shared_ptr<T> lock() const // never throws { #if defined(BOOST_HAS_THREADS) // optimization: avoid throw overhead if(expired()) { return wnd_shared_ptr<element_type>(); } try { return wnd_shared_ptr<element_type>(*this); } catch(bad_wnd_weak_ptr const &) { // Q: how can we get here? // A: another thread may have invalidated r after the use_count test above. return wnd_shared_ptr<element_type>(); } #else // optimization: avoid try/catch overhead when single threaded return expired()? wnd_shared_ptr<element_type>(): wnd_shared_ptr<element_type>(*this); #endif } And lets look at expired() bool expired() const // never throws { return pn.use_count() == 0; } Now, see what I mean? Best, John -- John Torjo -- john@torjo.com Contributing editor, C/C++ Users Journal -- "Win32 GUI Generics" -- generics & GUI do mix, after all -- http://www.torjo.com/win32gui/ -- v1.4 - save_dlg - true binding of your data to UI controls! + easily add validation rules (win32gui/examples/smart_dlg)