HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Implementing a shared_ptr class in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementingclassshared_ptr

Problem

I'm trying to write my own shared_ptr/weak_ptr implementation in C++. I have the following requirements:

I do NOT need support for the following:

  • multithreading (synchronisation)



  • support for polymorphic types as the templated type of the shared_ptr (such as shared_ptr Base*)



Reasons for wanting to write my own implementation:

  • need to supply a separate allocator for the control block



  • need to reduce the size of the control block (the standard version has a very large control block due to its support for multithreading and polymorphism among other things)



Concerns:

  • I'm worried about using my current implementation in production code (need some suggestions on how best to thoroughly test it)



  • I'm concerned I may have left out important features from my implementation



The following is what I've written so far (compilable in a C++11 compliant compiler, with main function and example):

```
#include
#include

struct shared_ptr_control_base
{
virtual ~shared_ptr_control_base() { }

void decrement_count_shared() noexcept { m_shared--; }
void increment_count_shared() noexcept { m_shared++; }

void decrement_count_weak() noexcept { m_weak--; }
void increment_count_weak() noexcept { m_weak++; }

virtual void destroy_shared(void*) noexcept = 0;
virtual void destruct() noexcept = 0;
virtual shared_ptr_control_base* create() const = 0;

uint32_t m_shared = 1;
uint32_t m_weak = 0;
};

template struct shared_ptr_control_derived : shared_ptr_control_base
{
shared_ptr_control_derived() = delete;
shared_ptr_control_derived(AllocatorType a_allocator) : m_allocator(a_allocator) { }

shared_ptr_control_derived* create() const
{
auto l_alloctor = std::allocator>();
auto l_p = l_alloctor.allocate(1);
l_alloctor.construct(l_p, *this);
return l_p;
}

void destroy_shared(void* a_pointer) noe

Solution

Big thing you have to watch are exceptions that will cause you to leak.

auto l_p = l_alloctor.allocate(1);
    l_alloctor.construct(l_p, *this);   // What happens if this throws?
    return l_p;

    // Need to make sure you re-clain the memory

    auto l_p = l_alloctor.allocate(1);
    try {
        l_alloctor.construct(l_p, *this);
    }
    catch(..) {
        l_alloctor.deallocate(l_p);
        throw;
    }
    return l_p;


The std::allocator call construct() does not need two arguments.

l_alloctor.construct(l_p, *this);

    // This results in a call to:
    new (lp) SharedType(std::forward>(*this));


This means the type you are sharing must have a constructor that takes an allocator as a parameter. You don't need to do this (since C++11). You only need to pass the pointer (which I think is what you actually want).

l_alloctor.construct(l_p); // See: http://en.cppreference.com/w/cpp/memory/allocator/construct


Why are you creating allocators on each function call?

auto l_alloctor = std::allocator>();


This works for simple allocators that use the standard C++ heap. But some allocators keep state (pool allocators). You must use the same allocator object to allocate/create/delete/de-allocate. And you already have a local allocator object stored locally m_allocator so use that.

Your create() and destruct() usage are not going to work well because they don't necessarily use the same allocator object (as the allocator is copied). SO you need to re-work all the use scenarios for this. I would personally make these two methods static.

Code Snippets

auto l_p = l_alloctor.allocate(1);
    l_alloctor.construct(l_p, *this);   // What happens if this throws?
    return l_p;

    // Need to make sure you re-clain the memory

    auto l_p = l_alloctor.allocate(1);
    try {
        l_alloctor.construct(l_p, *this);
    }
    catch(..) {
        l_alloctor.deallocate(l_p);
        throw;
    }
    return l_p;
l_alloctor.construct(l_p, *this);

    // This results in a call to:
    new (lp) SharedType(std::forward<shared_ptr_control_derived<T,A>>(*this));
l_alloctor.construct(l_p); // See: http://en.cppreference.com/w/cpp/memory/allocator/construct
auto l_alloctor = std::allocator<shared_ptr_control_derived<SharedType, AllocatorType>>();

Context

StackExchange Code Review Q#155966, answer score: 4

Revisions (0)

No revisions yet.