patterncppMinor
Implementing a shared_ptr class in C++
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:
Reasons for wanting to write my own implementation:
Concerns:
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
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.
The
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).
Why are you creating allocators on each function call?
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
Your
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/constructWhy 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/constructauto l_alloctor = std::allocator<shared_ptr_control_derived<SharedType, AllocatorType>>();Context
StackExchange Code Review Q#155966, answer score: 4
Revisions (0)
No revisions yet.