My shared_ptr implementation

Question

I would like to ask for a code review of my shared_ptr<T> implementation for design, code style. Any nitpicks will extremely useful for me. The features supported by my implementation are as follows:

make_shared<T> for a scalar-type T that performs no more than a single memory allocation.
Support for both scalar-types T and C-style arrays T[].
Support for custom deleters (type-erased)

Git Repo:

shared_ptr.h

#include <atomic>
#include <cassert>
#include <format>
#include <iostream>
#include <memory>
#include <utility>

namespace dev {

template<typename T>
class shared_ptr_base
{
  public:
    using value_type = T;
    using pointer = T*;
    using const_pointer = const T*;
    using reference = T&;
    using const_reference = const T&;

  protected:
    struct destroy_wrapper // RAII class
    {
        template<typename Deleter>
        explicit destroy_wrapper(Deleter&& deleter)
          : m_destroyer_ptr{ new destroyer<Deleter>(std::forward<Deleter>(deleter)) }
        {
        }

        destroy_wrapper(destroy_wrapper&& other) noexcept
          : m_destroyer_ptr{ std::exchange(other.m_destroyer_ptr, nullptr) }
        {
        }

        void operator()(pointer ptr)
        {
            (*m_destroyer_ptr)(ptr); // Virtual polymorphism
        }

        struct destroyer_base
        {
            virtual void operator()(pointer ptr) = 0;
            virtual ~destroyer_base() = default;
        };

        template<typename Deleter>
        struct destroyer : public destroyer_base
        {
            explicit destroyer(Deleter deleter)
              : destroyer_base()
              , m_deleter{ deleter }
            {
            }

            // destroy_wrapper is a wrapper over a unique_ptr<destroyer_base>.
            // It is intended to be ONLY move-constructible.

            void operator()(pointer ptr) override { m_deleter(ptr); }

            Deleter m_deleter;
        };

        ~destroy_wrapper() {}
        std::unique_ptr<destroyer_base> m_destroyer_ptr;
    };

    struct control_block_base
    {
        std::atomic<unsigned long long> m_ref_count;
        destroy_wrapper m_destroy_wrapper;

        /**
         * @brief Default constructor
         */
        control_block_base()
          : control_block_base(0u, destroy_wrapper(std::default_delete<T>()))
        {
        }

        explicit control_block_base(destroy_wrapper&& wrapper)
          : control_block_base(0u, std::move(wrapper))
        {
        }

        explicit control_block_base(unsigned long long ref_count,
                                    destroy_wrapper&& wrapper)
          : m_destroy_wrapper{ std::move(wrapper) }
        {
            m_ref_count.store(ref_count);
        }

        /**
         * @brief helper function to increment the object reference count
         */
        void increment() { m_ref_count.fetch_add(1u); }

        /**
         * @brief helper function to decrement the object reference count
         */
        auto decrement()
        {
            auto result = m_ref_count.fetch_sub(1u);
            return result;
        }

        // There is no use-case for copying control blocks of a shared_ptr<T>
        // instance. For safety, I delete these functions.
        control_block_base(const control_block_base&) = delete;
        control_block_base& operator=(const control_block_base&) = delete;
        control_block_base(control_block_base&&) = delete;
        control_block_base& operator=(control_block_base&&) = delete;

        /**
         * @brief Return the object reference count
         */
        [[nodiscard]] unsigned long long use_count() const noexcept
        {
            return m_ref_count.load();
        }

        virtual void release_shared(T*) = 0;
        virtual ~control_block_base() {}
    };

    struct control_block : public control_block_base
    {
        // pointer m_object_ptr;

        control_block()
          : control_block_base()
        {
        }

        explicit control_block(unsigned long long ref_count, destroy_wrapper&& wrapper)
          : control_block_base(ref_count, std::move(wrapper))
        {
        }

        explicit control_block(destroy_wrapper&& wrapper)
          : control_block_base(std::move(wrapper))
        {
        }

        void release_shared(T* raw_underlying_ptr) override
        {
            if (--this->m_ref_count == 0) {
                control_block_base::m_destroy_wrapper(raw_underlying_ptr);
                delete this;
            }
        }

        ~control_block() {}
    };

    /**
     * @brief This is a specialized version of %control_block_base, where the managed
     * object resides within the control block itself.
     */
    struct control_block_with_storage : public control_block_base
    {
        T m_object;

        /**
         * @brief Instantiates the managed object by perfectly forwarding
         * the constructor args.
         */
        template<typename... Args>
        explicit control_block_with_storage(Args&&... args)
          : control_block_base{ 1u, destroy_wrapper(std::default_delete<T>()) }
          , m_object(std::forward<Args>(args)...)
        {
        }

        /**
         * @brief C'tor that accepts a destroyer object and perfectly
         * forwards the constructor args
         */
        template<typename... Args>
        explicit control_block_with_storage(destroy_wrapper&& wrapper, Args&&... args)
          : control_block_base{ 1u, std::move(wrapper) }
          , m_object(std::forward<Args>(args)...)
        {
        }

        void release_shared(T*) override
        {
            if (--control_block_base::m_ref_count == 0) {
                delete this;
            }
        }

        T* get() { return &m_object; }
    };

  public:
    /**
     * @brief Default constructor - an empty shared_ptr
     */
    shared_ptr_base()
      : shared_ptr_base(nullptr)
    {
    }

    /**
     * @brief shared_ptr
     */
    shared_ptr_base(std::nullptr_t)
      : m_raw_underlying_ptr{ nullptr }
      , m_control_block_ptr{ new control_block() }
    {
    }

    /**
     * @brief Constructor that takes a raw pointer. Takes
     * ownership of the pointee.
     */
    explicit shared_ptr_base(pointer ptr)
      : m_raw_underlying_ptr{ ptr }
      , m_control_block_ptr{ nullptr }
    {
        // The child-classes will allocate the control_block
    }

    /**
     * @brief Constructor that takes a raw pointer and a custom deleter. Takes
     * ownership of the pointee.
     */
    explicit shared_ptr_base(T* ptr, destroy_wrapper&& destroyer)
      : m_raw_underlying_ptr{ ptr }
      , m_control_block_ptr{ nullptr }
    {
        try {
            if (ptr) {
                m_control_block_ptr = new control_block(1u, std::move(destroyer));
            } else {
                m_control_block_ptr = new control_block(std::move(destroyer));
            }

        } catch (std::exception& ex) {
            destroyer(ptr);
            throw ex;
        }
    }

    explicit shared_ptr_base(T* ptr, control_block_base* cb)
      : m_raw_underlying_ptr{ ptr }
      , m_control_block_ptr{ cb }
    {
    }

    /**
     * @brief Copy constructor. Models shared co-ownership of the resource
     * semantics.
     */
    shared_ptr_base(const shared_ptr_base& other)
      : m_raw_underlying_ptr{ other.m_raw_underlying_ptr }
      , m_control_block_ptr{ other.m_control_block_ptr }
    {
        if (m_control_block_ptr)
            m_control_block_ptr->increment(); // Atomic pre-increment
    }

    /**
     * @brief Move constructor. Represents transfer of ownership.
     */
    shared_ptr_base(shared_ptr_base&& other) noexcept
      : m_raw_underlying_ptr{ std::exchange(other.m_raw_underlying_ptr, nullptr) }
      , m_control_block_ptr{ std::exchange(other.m_control_block_ptr, nullptr) }
    {
    }

    /**
     * @brief Swaps two shared-pointer objects member-by-member.
     */
    void swap(shared_ptr_base& other) noexcept
    {
        std::swap(m_raw_underlying_ptr, other.m_raw_underlying_ptr);
        std::swap(m_control_block_ptr, other.m_control_block_ptr);
    }

    friend void swap(shared_ptr_base& lhs, shared_ptr_base& rhs) noexcept
    {
        lhs.swap(rhs);
    }

    /**
     * @brief Copy assignment operator. Release the currently held resource
     * and become a shared co-owner of the resource specified by user-supplied
     * argument @a other.
     */
    shared_ptr_base& operator=(const shared_ptr_base& other)
    {
        shared_ptr_base{ other }.swap(*this);
        return *this;
    }

    /**
     * @brief Move assignment operator. Release the currently held resource.
     * Transfer ownership of the resource.
     */
    shared_ptr_base& operator=(shared_ptr_base&& other)
    {
        shared_ptr_base{ std::move(other) }.swap(*this);
        return *this;
    }

    /**
     * @brief Destructor
     */
    ~shared_ptr_base()
    {
        if (m_raw_underlying_ptr) {
            m_control_block_ptr->release_shared(m_raw_underlying_ptr);
            m_raw_underlying_ptr = nullptr;
        }
    }

    // Pointer-like functions
    /**
     * @brief Returns the raw underlying pointer.
     */
    [[nodiscard]] pointer get() { return m_raw_underlying_ptr; }

    /**
     * @brief Returns a %pointer-to-const
     */
    [[nodiscard]] const_pointer get() const { return m_raw_underlying_ptr; }

    /**
     * @brief Returns a reference to the managed object
     */
    [[nodiscard]] reference operator*() noexcept { return *m_raw_underlying_ptr; }

    /**
     * @brief Returns a reference-to-const.
     */
    [[nodiscard]] const_reference operator*() const noexcept
    {
        return *m_raw_underlying_ptr;
    }

    /**
     * @brief Implementation of the indirection operator
     */
    [[nodiscard]] pointer operator->() noexcept { return m_raw_underlying_ptr; }

    /**
     * @brief Implementation of the indirection operator
     */
    [[nodiscard]] const_pointer operator->() const noexcept
    {
        return m_raw_underlying_ptr;
    }

    /**
     * @brief Spaceship operator.
     */
    friend auto operator<=>(const shared_ptr_base& lhs, const shared_ptr_base& rhs)
    {
        return lhs.m_raw_underlying_ptr <=> rhs.m_raw_underlying_ptr;
    }

    /**
     * @brief Equality comparison operator
     */
    [[nodiscard]] bool operator==(const shared_ptr_base& other) const noexcept
    {
        return (m_raw_underlying_ptr == other.m_raw_underlying_ptr);
    }

    [[nodiscard]] bool operator==(std::nullptr_t) const noexcept
    {
        return m_raw_underlying_ptr == nullptr;
    }

    /**
     * @brief Returns the reference count of the managed object
     */
    [[nodiscard]] unsigned long long use_count() const noexcept
    {
        if (m_control_block_ptr)
            return m_control_block_ptr->use_count();
        else
            return 0;
    }

    template<typename... Args>
    shared_ptr_base(Args&&... args)
    {
        /* Perform a single heap memory allocation */
        control_block_with_storage* cb = new control_block_with_storage(
          destroy_wrapper(std::default_delete<T>()), (std::forward<Args>(args))...);
        m_raw_underlying_ptr = cb->get();
        m_control_block_ptr = cb;
    }

    void reset_base(T* ptr, destroy_wrapper&& wrapper)
    {
        if (m_raw_underlying_ptr != ptr) {
            if (--m_control_block_ptr->m_ref_count == 0) {
                m_control_block_ptr->m_destroy_wrapper(m_raw_underlying_ptr);
                m_control_block_ptr->m_ref_count.store(1u);
            } else {
                // Multiple ownership
                m_control_block_ptr = new control_block(1u, std::move(wrapper));
            }
            shared_ptr_base<T>::m_raw_underlying_ptr = ptr;
        }
    }

  protected:
    T* m_raw_underlying_ptr;
    control_block_base* m_control_block_ptr;
};

template<typename T>
class shared_ptr : public shared_ptr_base<T>
{
  public:
    using control_block = shared_ptr_base<T>::control_block;
    using destroy_wrapper = typename shared_ptr_base<T>::destroy_wrapper;
    shared_ptr()
      : shared_ptr_base<T>()
    {
    }

    shared_ptr(std::nullptr_t)
      : shared_ptr_base<T>(nullptr)
    {
    }

    explicit shared_ptr(T* ptr)
      : shared_ptr_base<T>(ptr)
    {

        try {
            shared_ptr_base<T>::m_control_block_ptr =
              new control_block(1u, destroy_wrapper(std::default_delete<T>()));
        } catch (std::exception& ex) {
            delete ptr;
            // We may want to log the exception here
            throw ex;
        }
    }

    template<typename Deleter>
    explicit shared_ptr(T* ptr, Deleter deleter)
      : shared_ptr_base<T>(ptr, destroy_wrapper(deleter))
    {
    }

    template<typename... Args>
    explicit shared_ptr(Args... args)
      : shared_ptr_base<T>(std::forward<Args>(args)...)
    {
    }

    /**
     * @brief Replaces the managed object.
     */
    void reset(T* ptr)
    {
        shared_ptr_base<T>::reset_base(ptr, destroy_wrapper(std::default_delete<T>()));
    }
};

template<typename T>
class shared_ptr<T[]> : public shared_ptr_base<T>
{
  public:
    using control_block = shared_ptr_base<T>::control_block;
    using destroy_wrapper = typename shared_ptr_base<T>::destroy_wrapper;

    shared_ptr()
      : shared_ptr(nullptr)
    {
    }
    shared_ptr(std::nullptr_t)
      : shared_ptr_base<T>{ nullptr, destroy_wrapper(std::default_delete<T[]>()) }
    {
    }

    explicit shared_ptr(T* ptr)
      : shared_ptr_base<T>(ptr)
    {
        try {
            shared_ptr_base<T>::m_control_block_ptr =
              new control_block(1u, destroy_wrapper(std::default_delete<T[]>()));
        } catch (std::exception& ex) {
            delete[] ptr;
            // We may want to log the exception here
            throw ex;
        }
    }

    template<typename Deleter = std::default_delete<T[]>>
    explicit shared_ptr(T* ptr, Deleter deleter)
      : shared_ptr_base<T>(ptr, destroy_wrapper(deleter))
    {
    }

    template<typename... Args>
    explicit shared_ptr(Args... args)
      : shared_ptr_base<T[]>(std::forward<Args>(args)...)
    {
    }

    /**
     * @brief Replaces the managed object.
     */
    void reset(T* ptr)
    {
        shared_ptr_base<T>::reset_base(ptr, destroy_wrapper(std::default_delete<T[]>()));
    }

    T& operator[](int n) { return shared_ptr_base<T>::m_raw_underlying_ptr[n]; }
};

/**
 * @brief %make_shared is a utility function that accepts constructor
 * args and perfoms a single heap memory allocation for both the managed resource
 * and the control block.
 */
template<typename T, typename... Args>
shared_ptr<T> // Single-object version
make_shared(Args&&... args)
{
    return shared_ptr<T>(std::forward<Args>(args)...);
}

// TODO: Implement the array-version of make_shared<T[]>() available since C++20

} // namespace dev

shared_ptr_test.cpp

#include "shared_ptr.h"
#include <atomic>
#include <gtest/gtest.h>
#include <thread>

// make_shared<T>(constructor_args) test
TEST(SharedPtrTest, MakeSharedTest)
{
    struct Point2D
    {
        double x;
        double y;

        Point2D(double x_, double y_)
          : x{ x_ }
          , y{ y_ }
        {
        }

        ~Point2D() = default;
    };

    dev::shared_ptr<Point2D> sptr = dev::make_shared<Point2D>(3.0, 5.0);
    EXPECT_NE(sptr, nullptr);
    EXPECT_EQ(sptr->x, 3.0);
    EXPECT_EQ(sptr->y, 5.0);
    EXPECT_EQ(sptr.use_count(), 1);
}

TEST(SharedPtrTest, ResetSharedPtr)
{
    struct X
    {
        X(int n_)
          : n(n_)
        {
        }

        int get_n() { return n; }

        ~X() = default;
        int n;
    };

    dev::shared_ptr<X> sptr(new X(100));
    EXPECT_NE(sptr, nullptr);
    EXPECT_EQ(sptr.use_count(), 1);
    EXPECT_EQ(sptr->get_n(), 100);

    // Reset the shared_ptr handing it a fresh instance of X
    sptr.reset(new X(200));
    EXPECT_NE(sptr, nullptr);
    EXPECT_EQ(sptr.use_count(), 1);
    EXPECT_EQ(sptr->get_n(), 200);
}

TEST(SharedPtrTest, ResetSharedPtrMultipleOwnership)
{
    struct X
    {
        X(int n_)
          : n(n_)
        {
        }

        int get_n() { return n; }

        ~X() = default;
        int n;
    };

    dev::shared_ptr<X> sptr1(new X(100));
    dev::shared_ptr sptr2 = sptr1;
    dev::shared_ptr sptr3 = sptr2;

    EXPECT_EQ(sptr1->get_n(), 100);
    EXPECT_EQ(sptr2->get_n(), 100);
    EXPECT_EQ(sptr3->get_n(), 100);
    EXPECT_EQ(sptr1.use_count(), 3);

    // Reset the shared_ptr sptr1. Hand it a new instance of X.
    // The old instance will stay shared between sptr2 and sptr3.
    sptr1.reset(new X(200));
    EXPECT_EQ(sptr1->get_n(), 200);
    EXPECT_EQ(sptr2->get_n(), 100);
    EXPECT_EQ(sptr3->get_n(), 100);
    EXPECT_EQ(sptr1.use_count(), 1);
    EXPECT_EQ(sptr2.use_count(), 2);
}

TEST(SharedPtrTest, ResetArrayVersion)
{
    int* a = new int[3];
    a[0] = 1;
    a[1] = 2;
    a[2] = 3;
    int* b = new int[3];
    b[0] = 4;
    b[1] = 5;
    b[2] = 6;

    dev::shared_ptr<int[]> sptr1(a);
    dev::shared_ptr<int[]> sptr2(sptr1);
    dev::shared_ptr<int[]> sptr3(sptr2);

    EXPECT_EQ(sptr1.use_count(), 3);
    EXPECT_EQ(sptr1[0], 1);
    EXPECT_EQ(sptr1[1], 2);
    EXPECT_EQ(sptr1[2], 3);

    sptr1.reset(b);
    EXPECT_EQ(sptr1.use_count(), 1);
    EXPECT_EQ(sptr2.use_count(), 2);
    EXPECT_EQ(sptr1[0], 4);
    EXPECT_EQ(sptr1[1], 5);
    EXPECT_EQ(sptr1[2], 6);

    EXPECT_EQ(sptr2[0], 1);
    EXPECT_EQ(sptr2[1], 2);
    EXPECT_EQ(sptr2[2], 3);
}

TEST(SharedPtrTest, ParametrizedCTorTestScalarVersion)
{
    auto ptr = new int(17);
    dev::shared_ptr<int> s_ptr{ ptr };
    EXPECT_EQ(*s_ptr, 17);
    EXPECT_NE(s_ptr.get(), nullptr);
    EXPECT_EQ(s_ptr.get(), ptr);
}

TEST(SharedPtrTest, ParametrizedCTorTestArrayVersion)
{
    auto ptr = new int[10]();
    for (int i{ 0 }; i < 10; ++i) {
        ptr[i] = i + 1;
    }
    dev::shared_ptr<int[]> s_ptr(ptr);
    EXPECT_NE(s_ptr, nullptr);
    EXPECT_EQ(s_ptr.get(), ptr);
    for (int i{ 0 }; i < 10; ++i) {
        EXPECT_EQ(ptr[i], i + 1);
        EXPECT_EQ(s_ptr[i], ptr[i]);
    }
}

TEST(SharedPtrTest, RefCountingTest)
{
    int* raw_ptr = new int(42);
    {
        dev::shared_ptr<int> ptr1{ raw_ptr };
        EXPECT_EQ(ptr1.use_count(), 1);
        EXPECT_EQ(ptr1.get(), raw_ptr);
        {
            dev::shared_ptr ptr2 = ptr1;
            EXPECT_EQ(ptr1.use_count(), 2);
            EXPECT_EQ(ptr1.get(), raw_ptr);
            {
                dev::shared_ptr ptr3 = ptr2;
                EXPECT_EQ(ptr1.use_count(), 3);
                EXPECT_EQ(ptr1.get(), raw_ptr);
            }
            EXPECT_EQ(ptr1.use_count(), 2);
            EXPECT_EQ(ptr1.get(), raw_ptr);
        }
        EXPECT_EQ(ptr1.use_count(), 1);
        EXPECT_EQ(ptr1.get(), raw_ptr);
    }
}

TEST(SharedPtrTest, RefCountingTestArrayVersion)
{
    int* raw_ptr = new int[5];
    {
        dev::shared_ptr<int[]> ptr1{ raw_ptr };
        EXPECT_EQ(ptr1.use_count(), 1);
        EXPECT_EQ(ptr1.get(), raw_ptr);
        {
            dev::shared_ptr ptr2 = ptr1;
            EXPECT_EQ(ptr1.use_count(), 2);
            EXPECT_EQ(ptr1.get(), raw_ptr);
            {
                dev::shared_ptr ptr3 = ptr2;
                EXPECT_EQ(ptr1.use_count(), 3);
                EXPECT_EQ(ptr1.get(), raw_ptr);
            }
            EXPECT_EQ(ptr1.use_count(), 2);
            EXPECT_EQ(ptr1.get(), raw_ptr);
        }
        EXPECT_EQ(ptr1.use_count(), 1);
        EXPECT_EQ(ptr1.get(), raw_ptr);
    }
}

TEST(SharedPtrTest, MultithreadedConstructionAndDestructionTest)
{
    using namespace std::chrono_literals;
    dev::shared_ptr ptr{ new int(42) };
    std::atomic<bool> go{ false };
    EXPECT_EQ(ptr.use_count() == 1, true);

    std::thread t1([&] {
        dev::shared_ptr<int> ptr1 = ptr;
        while (!go.load())
            ;
        std::cout << "\nRef Count = " << ptr.use_count();
        std::this_thread::sleep_for(1s);
    });

    std::thread t2([&] {
        dev::shared_ptr<int> ptr2 = ptr;
        while (!go.load())
            ;
        std::cout << "\nRef Count = " << ptr.use_count();
        std::this_thread::sleep_for(1s);
    });

    std::this_thread::sleep_for(1s);
    go.store(true);
    t1.join();
    t2.join();
    EXPECT_EQ(ptr.use_count() == 1, true);
}

TEST(SharedPtrTest, CopyConstructorTest)
{
    /* Copy constructor */
    int* raw_ptr = new int(42);
    dev::shared_ptr<int> p1(raw_ptr);

    dev::shared_ptr<int> p2 = p1;
    EXPECT_EQ(p1.get(), raw_ptr);
    EXPECT_EQ(p2, p1);
    EXPECT_EQ(*p2, 42);
    EXPECT_EQ(p2.get(), raw_ptr);
}

TEST(SharedPtrTest, CopyConstructorTestArrayVersion)
{
    /* Copy constructor */
    int* raw_ptr = new int[3];
    raw_ptr[0] = 42;
    raw_ptr[1] = 5;
    raw_ptr[2] = 17;

    dev::shared_ptr<int[]> p1(raw_ptr);

    dev::shared_ptr p2{ p1 };
    EXPECT_EQ(p1.get(), raw_ptr);
    EXPECT_EQ(p2, p1);
    EXPECT_EQ(p2.get(), raw_ptr);

    EXPECT_EQ(p2[0], 42);
    EXPECT_EQ(p2[1], 5);
    EXPECT_EQ(p2[2], 17);
}

TEST(SharedPtrTest, MoveConstructorTest)
{
    /* Move constructor*/
    auto raw_ptr = new int(28);
    dev::shared_ptr<int> p1(raw_ptr);
    dev::shared_ptr<int> p2 = std::move(p1);
    dev::shared_ptr<int> p3 = std::move(p2);
    EXPECT_EQ(p1.get(), nullptr);
    EXPECT_EQ(p1.use_count(), 0);
    EXPECT_EQ(p2.get(), nullptr);
    EXPECT_EQ(p2.use_count(), 0);
    EXPECT_NE(p3, nullptr);
    EXPECT_EQ(p3.get(), raw_ptr);
    EXPECT_EQ(p3.use_count(), 1);
    EXPECT_EQ(*p3, 28);
}

TEST(SharedPtrTest, CopyAssignmentTest)
{
    /* Copy Assignment */
    dev::shared_ptr<double> p1(new double(2.71828));
    dev::shared_ptr<double> p2(new double(3.14159));

    EXPECT_EQ(*p2 == 3.14159, true);
    p2 = p1;
    EXPECT_EQ(p2.get() == p1.get(), true);
    EXPECT_EQ(*p2 == *p1, true);
}

TEST(SharedPtrTest, MoveAssignmentTest)
{
    /* Move Assignment */
    dev::shared_ptr<int> p1(new int(42));
    dev::shared_ptr<int> p2(new int(28));
    p2 = std::move(p1);
    EXPECT_EQ(p2.get() != nullptr, true);
    EXPECT_EQ(*p2 == 42, true);
}

/* swap() : swap the managed objects */
TEST(SharedPtrTest, SwapTest)
{
    int* first = new int(42);
    int* second = new int(17);

    dev::shared_ptr<int> p1(first);
    dev::shared_ptr<int> p2(second);

    swap(p1, p2);

    EXPECT_EQ(p2.get() == first && p1.get() == second, true);
    EXPECT_EQ(((*p1) == 17) && ((*p2) == 42), true);
}

// Observers
/* get() : Returns a pointer to the managed object or nullptr*/
TEST(SharedPtrTest, GetTest)
{
    double* resource = new double(0.50);
    dev::shared_ptr p(resource);

    EXPECT_EQ(p.get() == resource, true);
    EXPECT_EQ(*(p.get()) == 0.50, true);
}

// Pointer-like functions
TEST(SharedPtrTest, IndirectionOperatorTest)
{
    /* indirection operator* to dereference pointer to managed object,
       member access operator -> to call member function*/
    struct X
    {
        int _n;

        X() = default;
        X(int n)
          : _n{ n }
        {
        }
        ~X() = default;
        int foo() { return _n; }
    };

    dev::shared_ptr<X> ptr(new X(10));
    EXPECT_EQ((*ptr)._n == 10, true);
    EXPECT_EQ(ptr->foo() == 10, true);
}

// Custom deleter test
TEST(SharedPtrTest, CustomDeleterTest)
{
    struct Point2D
    {
        double x;
        double y;

        Point2D(double x_, double y_)
          : x{ x_ }
          , y{ y_ }
        {
        }

        ~Point2D() = default;
    };

    auto custom_deleter = [](Point2D* ptr) {
        std::cout << "\n" << "custom_deleter invoked";
        ptr->~Point2D();        // Call the destructor
        ::operator delete(ptr); // Deallocate memory
    };

    {
        dev::shared_ptr<Point2D> ptr1(new Point2D(3.0, 5.0), custom_deleter);
        EXPECT_EQ(ptr1.use_count(), 1);
        {
            dev::shared_ptr<Point2D> ptr2 = ptr1;
            EXPECT_EQ(ptr2.use_count(), 2);
        }

        EXPECT_EQ(ptr1.use_count(), 1);
    }
}

indi · Accepted Answer · 2025-07-21 02:32:46Z

Not going to do a full review, because I don’t review code from old language versions (C++17 is now 2½ versions old), but there is one thing I’d like to point out that is language-version agnostic.

Learning C++ by trying to re-implement part of the standard library is not generally a bad idea. It depends a lot on which part exactly you’re re-implementing; re-implementing std::vector, for example, is an absolutely terrible idea. But std::shared_ptr? Not bad at all. Challenging, sure, but you could learn a lot from the attempt.

However, the single most important thing to do when studying the standard library to learn C++—whether your goal is to re-implement it or not—is to study the API carefully.

C++ is not like most other languages that use the “benevolent dictator” model. Everything that gets added to C++ gets scrutinized by multiple technically-specialized committees, overseen by a coalition of several of the largest software development companies in the world, and multiple national bodies. It’s not a perfect system, not by a long shot, but most of the time, anything that gets in to the standard has been heavily reviewed, many, many, many times over, by dozens… and sometimes literally hundreds… of the top experts in the field. (And that’s just to mention the people actually in the standards committees; if you count all the outsiders who also review, comment, and suggest, then some of the more popular stuff has gone through tens of thousands of reviewers. std::shared_ptr would be one of those things that was massively and widely reviewed before being accepted.)

Which means that whatever was added—or not added— to std::shared_ptr’s interface was very carefully considered.

For example: Why did they bother to add a constructor for std::nullptr_t? The constructor that takes a single pointer (constructor #3 on the cppreference page) would handle that case. So why add the extra constructor? It wasn’t just for the hell of it. Every little thing added to the C++ standard creates extra maintenance burden; it’s not done lightly. There is a very specific reason why this extra constructor adds real technical value. Understanding that, and what value exactly it adds, is the real lesson you get from re-implementing std::shared_ptr.

And, in fact, there are a couple of reasons why std::shared_ptr needs a nullptr constructor. Unfortunately, you have failed to understand those reasons, and as a result, your shared_ptr does not need a nullptr constructor. Yes, it has one, but it’s pretty pointless; you could just as well define it as shared_ptr(std::nullptr_t) : shared_ptr{static_cast<T*>(nullptr)} {}, or not define it at all (except for the other problematic constructor, that I will be getting to shortly). That won’t work for std::shared_ptr, and understanding why is the real lesson to be learned here.

Put another way: The std::shared_ptr nullptr constructor is both constexpr and noexcept. Your shared_ptr nullptr constructor can be neither. Why is that?

Another thing you need to be cautious about is adding things to a standard-defined API. For example, this constructor:

    template<typename... Args>
    explicit shared_ptr(Args... args)
      : shared_ptr_base<T>(std::forward<Args>(args)...)
    {
    }

Now, putting aside that the arguments are not forwarding references—I’m just assuming they should be—you must have noticed that std::shared_ptr has no such constructor. Why do you think that is? It’s not like they didn’t think about it; they made std::make_shared(), after all. So why did they make std::make_shared() as a separate function if they could just as well make it a constructor, like you did?

I’m not giving away the answer; I just want to impress upon you that if the standard committee didn’t do a thing, they probably had a very good reason. As in, this constructor is extremely problematic. Think through why that would be the case.

If you need a hint, remember the fundamental hazard of including templates in an overload set: templates are greedy. They will match anything that does not match the other overloads exactly. Even mere const-ness matters. Adding a universal template to an overload set is generally a really, really bad idea.

If you still can’t see how this could blow up in your face, then just imagine a case of shared_ptr<Base> with a Derived pointer. For bonus points, imagine that Base has a constructor that can accept a Derived pointer.

Re-implementing standard library components is not a bad way to learn C++, if you’re careful about which components you choose. However, the real lesson is not in just doing. The real lesson is in understanding. If, after you’ve re-implemented std::shared_ptr, you can’t explain why it was designed with the interface it has, then you haven’t really learned much from the exercise.

Minor point: "overseen by a coalition of several of the largest software development companies in the world" understates the role of small companies and individuals. For example, back before we both retired, PJ Plauger and I were the technical backbone of Dinkumware, Ltd. Hardly a large company, but highly influential. — Pete Becker
– Pete Becker, Commented Jul 22 at 18:54
Truth. I don’t have enough nice things to say about Dinkumware. — indi
– indi, Commented Jul 22 at 19:36

G. Sliepen · Accepted Answer · 2025-07-22 19:31:01Z

5

Suboptimal handling of the destroyer

std::shared_ptr is not an easy thing to implement, but you are making it extra hard with the way you handle the destroyer. That's three extra classes that's basically doing nothing except storing the destroyer in a std::unique_ptr!

Furthermore, the control block can either be separated from the object, or hold the object itself. That is a nice optimization, but why not use that for the destroyer as well? You could make control_block and control_block_with_storage templated on the deleter type, and then store the deleter by value.

Use `std::size_t` for refcounts

unsigned long long is the wrong type to use for reference counts. Instead, use std::size_t. It is guaranteed to be big enough to count any possible number of shared pointers you have to the same object. There is no such guarantee for unsigned long long; although it's unlikely that would be too small, it is more likely to be larger than necessary, and could even be large enough that a std::atomic<unsigned long long> would have to use a mutex internally to guarantee its atomicity.

`decrement()` is not used

It's a bit weird to see increment() and decrement() functions, but decrement() is not being used.

edited Jul 22 at 19:31

answered Jul 20 at 21:40

G. Sliepen

69.3k3 gold badges75 silver badges180 bronze badges

\$\begingroup\$ Your last point: "You can use std::memory_order_relaxed when incrementing and decrementing the reference count" is wrong. You can only use relaxed when incrementing because the compiler potentially moving reads of the managed data before the increment is OK, since the original pointer you're copying must stay valid until the copy is complete to avoid data races. Decrements on the other hand do need to be acquire-release (or release with an acquire inside the deletion branch) to avoid reads and writes to the managed data from moving past the destructor, which is obviously bad. \$\endgroup\$

user292503
– user292503

2025-07-21 14:23:53 +00:00
Commented Jul 21 at 14:23
\$\begingroup\$ Alternatively put: you need to ensure that all reads and writes to managed data happen while at least one copy of a shared_ptr object is holding a reference to the managed data. Before the constructor you have that guarantee (the shared_ptr you're copying must exist between when you first see it and when you're done copying it) but after the destructor you have no such guarantee. Thus the constructor (increment) can allow accesses to move before it, but the destructor (decrement) can't permit accesses to move after it and requires a stronger memory ordering. \$\endgroup\$

user292503
– user292503

2025-07-21 14:28:39 +00:00
Commented Jul 21 at 14:28
2

\$\begingroup\$ @MatthieuM. unique_ptr works because the same thread reads and destructs it, thus no synchronization is needed. The problem with shared_ptr is if one imagines a program that did: { copy; read; destruct; } on a shared pointer, which when spread over two threads A and B could be executed as: { (A)copy; (B)copy; (A)read; (B)destruct; (A)destruct; (B)read; }. Because the destruct in B doesn't destroy the object (A still holds a reference) it's fine to delay the read on B if the decrement permits it. When A destructs the object later the delayed read on B will then read a deleted object, hence UB. \$\endgroup\$

user292503
– user292503

2025-07-21 15:55:29 +00:00
Commented Jul 21 at 15:55
1

\$\begingroup\$ @MatthieuM. It's required for two reasons: firstly because the standard says that releases need to be paired with acquires, and vice versa. The second (more practical) reason is to keep the content of the destructor (not the end-of-life, but just some code from a user defined destructor i.e. the write to i in struct foo { int i; ~foo(){ i = 0; } }) from moving before the decrement. This could happen if a particularly smart compiler could work out that one thread would always reduce the reference count to 0, and thus remove the branch, allowing the destructor to be moved before the decrement. \$\endgroup\$

user292503
– user292503

2025-07-21 16:44:35 +00:00
Commented Jul 21 at 16:44
1

\$\begingroup\$ @Ifier: Thanks (again) for the explanation. I missed the issue that while the memory release shouldn't be executed speculatively, the destructor certainly could, and this may cause a data-race indeed. I would expect that the acquire semantics can be obtained by using atomic_thread_fence within the branch, with the load in the Release atomic decrement counting as the "Relaxed" load for the purpose of the Acquire fence. \$\endgroup\$

Matthieu M.
– Matthieu M.

2025-07-21 17:22:58 +00:00
Commented Jul 21 at 17:22

| Show 3 more comments

Stack Exchange Network

My shared_ptr implementation

2 Answers 2

Suboptimal handling of the destroyer

Use `std::size_t` for refcounts

`decrement()` is not used

You must log in to answer this question.

Hot Network Questions

My shared_ptr implementation

2 Answers 2

Suboptimal handling of the destroyer

Use std::size_t for refcounts

decrement() is not used

You must log in to answer this question.

Related

Hot Network Questions

Use `std::size_t` for refcounts

`decrement()` is not used