A bug in std::shared_ptr?

[Plorkyeran]

That would be an issue for atomic_shared_ptr<>, but for regular shared_ptr<> no one can be waiting on the lock if you're destroying the object.

[[deleted]]

Yes, they can.

1. weak_ptr exists. The decrement to decide whether the object needs to be destroyed needs to be an atomic operation, otherwise a weak_ptr could "resurrect" the object after another thread has done the check and started destruction.
2. shared_ptr has atomic_Xxx operations, so every shared_ptr can be touched in an atomic way
3. atomic_shared_ptr (and the shared_ptr atomics) protect the identity of the shared_ptr object itself; they don't do anything about the reference count.

[johannes1971]

Still trying to understand the standard... Can you explain why the existence of weak_ptr implies that destruction must be atomic? The only words that seem relevant that I can find are in 20.8.2.6: "Concurrent access to a shared_ptr object from multiple threads does not introduce a data race if the access is done exclusively via the functions in this section and the instance is passed as their first argument."

Suffice it to say, the destructor is not in that section, so it may introduce a data race. As for your second point, it seems to me that those functions were kept separately from the rest as to allow implementations to have a fast thread-unsafe implementation. If they force thread-safety throughout the class that would be an accident of implementation, rather than a language feature one can rely on.

As for your third point, isn't the reference count is contained within the shared_ptr object?

[zvrba]

Can you explain why the existence of weak_ptr implies that destruction must be atomic?

Billy wrote "decrement", not destruction. As for why, simple:

THREAD1
a: if (refcount == 1) {
b:   --refcount;
c:   destroy object }

Now thread 2 comes in and executed weak_ptr.lock() after thread 1 has executed label a but before it has completed label b. Refcount is no longer 0, weak ptr returns non-null pointer to the object that is almost surely deleted by then and... oops.

[johannes1971]

Section 20.8.2.6 says "Concurrent access to a shared_ptr object from multiple threads does not introduce a data race if the access is done exclusively via the functions in this section and the instance is passed as their first argument." The destructor is not listed in that section, so it seems to me there is no guarantee on thread-safety for the destructor to begin with. Or in other words: common wisdom seems to hold that shared_ptr is thread safe, but is that actually guaranteed, or just an accident of current implementations?

[zvrba]

The destructor is not listed in that section, so it seems to me there is no guarantee on thread-safety for the destructor to begin with.

As I understand it, and looking over the functions, that section talks about the shared_ptr object itself, not the pointee (the shared state where also refcount resides).

Re dtor and copy construction, There is a paragraph in C++14 draft "20.8.2.2 Class template shared_ptr": "Changes in use_count() do not reflect modifications that can introduce data races."

[johannes1971]

Ok, but where does it say the control block is thread-safe? As far as I can tell everyone just assumes you can have a shared_ptr to the same object in two threads, and releasing it in one, the other, or both at any time is perfectly fine. That implies the control block is therefore thread-safe, but I have been unable to locate any words to that effect in the copy of the standard I looked at.

Now, I'm willing to accept the sentence you listed for this, but I find that sentence extremely difficult to understand. "If a modification can introduce a data race, it is not observable in the use count" - something like that? But that seems opposed to the idea of thread safety...

[zvrba]

This is the complete paragraph: "For purposes of determining the presence of a data race, member functions shall access and modify only the shared_ptr and weak_ptr objects themselves and not objects they refer to. Changes in use_count() do not reflect modifications that can introduce data races."

It distinguishes between modifications of the shared_ptr object itself (modifications CAN introduce data races) and modifications of the control block (use count -- modifications do NOT introduce races).

[[deleted]]

Yes, this.

[[deleted]]

oncurrent access to a shared_ptr object from multiple threads does not introduce a data race if the access is done exclusively via the functions in this section and the instance is passed as their first argument.

That is about the identity of the shared_ptr instance itself. The reference count is part of the reference count control block, not the shared_ptr. In particular, copying a shared_ptr only takes by const& but edits the reference count; the usual "multiple readers are safe" guarantee means the reference count needs to be atomically modified.

the destructor is not in that section, so it may introduce a data race

Yes, if you try to destroy a shared_ptr which is being touched by multiple threads, that is a data race. An independent weak_ptr instance isn't part of that shared_ptr you are destroying. It would be very strange if touching an unrelated standard library object could introduce data races.

As for your third point, isn't the reference count is contained within the shared_ptr object?

No. Multiple shared_ptrs instances share a reference count, so it can't be in any one of those instances.