Thread-safety of reference count in std::shared_ptr

Question

Looking at this implementation of std::shared_ptr https://thecandcppclub.com/deepeshmenon/chapter-10-shared-pointers-and-atomics-in-c-an-introduction/781/ :

Question 1 : I can see that we're using std::atomic<int*> to store the pointer to the reference count associated with the resource being managed. Now, in the destructor of the shared_ptr, we're changing the value of the ref-count itself (like --(*reference_count)). Similarly, when we make a copy of the shared_ptr, we increment the ref-count value. However, in both these operations, we're not changing the value of the pointer to the ref-count but rather ref-count itself. Since the pointer to ref-count is the "atomic thing" here, I was wondering how would ++ / -- operations to the ref-count be thread-safe? Is std::atomic implemented internally in a way such that in case of pointers, it ensures changes to the underlying object itself are also thread-safe?

Question 2 : Do we really need this nullptr check in default_deleter class before calling delete on ptr? As per Is it safe to delete a NULL pointer?, it is harmless to call delete on nullptr.

(1) Please don't link to code include it in the question. (2) Perhaps you should be looking at a live implementations of `std::shared_ptr` GCC, clang and MSVC are all available. — Richard Critten, Jun 12 '22 at 13:29
Not only does the linked page think it can prevent access to the page source and make it especially hard to browse, it also has an incorrect implementation. No point in arguing about the rationale in an implementation that doesn't work anyways: Neither move nor copy assignment even considers deleting an object, it must do this for a correct implementation of a shared pointer class, since you may be overriding the last shared pointer instance refering to an object. Furthermore the copy assignment doesn't consider self-assignment. — fabian, Jun 12 '22 at 13:51
@RichardCritten (1) I could not copy the code in this case because that link prevents one from copying anything but will keep this in mind for future also (2) Yeah I should have started with one of those links I guess (MS's STL implementation can be found here https://github.com/microsoft/STL/blob/main/stl/inc/memory for instance if it helps the next person looking at this answer). Thanks Sam / Matt / fabian for your responses! — not_that_guy123, Jun 12 '22 at 15:57

user17732522 · Accepted Answer · 2022-06-12T14:26:26.970

Question 1:

The implementation linked to is not thread-safe at all. You are correct that the shared reference counter should be atomic, not pointers to it. std::atomic<int*> here makes no sense.

Note that just changing std::atomic<int*> to std::atomic<int>* won't be enough to fix this either. For example the destructor is decrementing the reference count and checking it against 0 non-atomically. So another thread could get in between these two operations and then they will both think that they should delete the object causing undefined behavior.

As mentioned by @fabian in the comments, it is also far from a correct non-thread-safe shared pointer implementation. For example with the test case

{
    Shared_ptr<int> a(new int);
    Shared_ptr<int> b(new int);
    b = a;
}

it will leak the second allocation. So it doesn't even do the basics correctly.

Even more, in the simple test case

{
    Shared_ptr<int> a(new int);
}

it leaks the allocated memory for the reference counter (which it always leaks).

Question 2:

There is no reason to have a null pointer check there except to avoid printing the message. In fact, if we want to adhere to the standard's specification of std::default_delete for default_deleter, then at best it is wrong to check for nullptr, since that is specified to call delete unconditionally.

But the only possible edge case where this could matter is if a custom operator delete would be called that causes some side effect for a null pointer argument. However, it is anyway unspecified whether delete will call operator delete if passed a null pointer, so that's not practically relevant either.

Thread-safety of reference count in std::shared_ptr

1 Answers1