Skip to content

Commit c093c4d

Browse files
committed
- Fix memory leaks in new Cache
- Modify LinkedList to use C++11 constructs (allocator traits) - Properly forward arguments during Cache insertion to avoid multiple calls to move constructor
1 parent 0d93567 commit c093c4d

File tree

1 file changed

+39
-26
lines changed

1 file changed

+39
-26
lines changed

include/nbl/core/containers/DoublyLinkedList.h

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ struct alignas(void*) SDoublyLinkedNode
3434
prev = invalid_iterator;
3535
next = invalid_iterator;
3636
}
37-
SDoublyLinkedNode(Value&& val) : data(std::move(val))
37+
template <typename... Args>
38+
SDoublyLinkedNode(Args&&... args) : data(std::forward<Args>(args)...)
3839
{
3940
prev = invalid_iterator;
4041
next = invalid_iterator;
@@ -69,6 +70,7 @@ class DoublyLinkedList
6970
{
7071
public:
7172
using allocator_t = allocator;
73+
using allocator_traits_t = std::allocator_traits<allocator_t>;
7274
using address_allocator_t = PoolAddressAllocator<uint32_t>;
7375
using node_t = SDoublyLinkedNode<Value>;
7476
using value_t = Value;
@@ -112,15 +114,15 @@ class DoublyLinkedList
112114
template <typename... Args>
113115
inline void emplaceFront(Args&&... args)
114116
{
115-
insertAt(reserveAddress(), value_t(std::forward<Args>(args)...));
117+
insertAt(reserveAddress(), std::forward<Args>(args)...);
116118
}
117119

118120
/**
119-
* @brief Resets list to initial state.
121+
* @brief Empties list: calls disposal function on and destroys every node in list, then resets state
120122
*/
121123
inline void clear()
122124
{
123-
disposeAll();
125+
destroyAll();
124126

125127
m_addressAllocator = address_allocator_t(m_reservedSpace, 0u, 0u, 1u, m_cap, 1u);
126128
m_back = invalid_iterator;
@@ -139,6 +141,8 @@ class DoublyLinkedList
139141
get(backNode->prev)->next = invalid_iterator;
140142
uint32_t temp = m_back;
141143
m_back = backNode->prev;
144+
if (m_back == invalid_iterator)
145+
m_begin = invalid_iterator;
142146
common_delete(temp);
143147
}
144148

@@ -189,9 +193,12 @@ class DoublyLinkedList
189193
return false;
190194
// Have to consider allocating enough space for list AND state of the address allocator
191195
const auto firstPart = core::alignUp(address_allocator_t::reserved_size(1u, newCapacity, 1u), alignof(node_t));
192-
void* newReservedSpace = reinterpret_cast<void*>(m_allocator.allocate(firstPart + newCapacity * sizeof(node_t)));
196+
// Allocator can only allocate in terms of nodes
197+
const size_t firstPartNodes = (firstPart + sizeof(node_t) - 1) / sizeof(node_t);
198+
const size_t newAllocationSize = firstPartNodes + newCapacity;
199+
void* newReservedSpace = reinterpret_cast<void*>(allocator_traits_t::allocate(m_allocator, newAllocationSize));
193200

194-
// Malloc failed, not possible to grow
201+
// Allocation failed, not possible to grow
195202
if (!newReservedSpace)
196203
return false;
197204

@@ -201,12 +208,12 @@ class DoublyLinkedList
201208
// Create new address allocator from old one
202209
m_addressAllocator = address_allocator_t(newCapacity, std::move(m_addressAllocator), newReservedSpace);
203210
// After address allocator creation we can free the old buffer
204-
m_allocator.deallocate(reinterpret_cast<node_t*>(m_reservedSpace));
211+
allocator_traits_t::deallocate(m_allocator, reinterpret_cast<node_t*>(m_reservedSpace), m_currentAllocationSize);
205212
m_cap = newCapacity;
206213
m_array = newArray;
207214
m_reservedSpace = newReservedSpace;
215+
m_currentAllocationSize = newAllocationSize;
208216

209-
210217
return true;
211218
}
212219

@@ -216,11 +223,15 @@ class DoublyLinkedList
216223
{
217224
// Have to consider allocating enough space for list AND state of the address allocator
218225
const auto firstPart = core::alignUp(address_allocator_t::reserved_size(1u, capacity, 1u), alignof(node_t));
219-
m_reservedSpace = reinterpret_cast<void*>(m_allocator.allocate(firstPart + capacity * sizeof(node_t)));
226+
// Allocator can only allocate in terms of nodes
227+
const size_t firstPartNodes = (firstPart + sizeof(node_t) - 1) / sizeof(node_t);
228+
m_currentAllocationSize = firstPartNodes + capacity;
229+
m_reservedSpace = reinterpret_cast<void*>(allocator_traits_t::allocate(m_allocator, m_currentAllocationSize));
220230
m_array = reinterpret_cast<node_t*>(reinterpret_cast<uint8_t*>(m_reservedSpace) + firstPart);
221231

222232
m_addressAllocator = address_allocator_t(m_reservedSpace, 0u, 0u, 1u, capacity, 1u);
223-
m_cap = capacity;
233+
// If allocation failed, create list with no capacity to indicate creation failed
234+
m_cap = m_reservedSpace ? capacity : 0;
224235
m_back = invalid_iterator;
225236
m_begin = invalid_iterator;
226237
}
@@ -250,11 +261,11 @@ class DoublyLinkedList
250261

251262
~DoublyLinkedList()
252263
{
253-
disposeAll();
264+
destroyAll();
254265
// Could be null if list was moved out of
255266
if (m_reservedSpace)
256267
{
257-
m_allocator.deallocate(reinterpret_cast<node_t*>(m_reservedSpace));
268+
allocator_traits_t::deallocate(m_allocator, reinterpret_cast<node_t*>(m_reservedSpace), m_currentAllocationSize);
258269
}
259270
}
260271

@@ -266,12 +277,14 @@ class DoublyLinkedList
266277
return addr;
267278
}
268279

269-
//create a new node which stores data at already allocated address,
270-
inline void insertAt(uint32_t addr, value_t&& val)
280+
//create a new node which stores data at already allocated address
281+
template <typename... Args>
282+
inline void insertAt(uint32_t addr, Args&&... args)
271283
{
272284
assert(addr < m_cap);
273285
assert(addr != invalid_iterator);
274-
SDoublyLinkedNode<Value>* n = new(m_array + addr) SDoublyLinkedNode<Value>(std::move(val));
286+
node_t* n = m_array + addr;
287+
allocator_traits_t::construct(m_allocator, n, std::forward<Args>(args)...);
275288
n->prev = invalid_iterator;
276289
n->next = m_begin;
277290

@@ -283,20 +296,18 @@ class DoublyLinkedList
283296
}
284297

285298
/**
286-
* @brief Calls disposal function on all elements of the list.
299+
* @brief Calls disposal function then destroys all elements in the list.
287300
*/
288-
inline void disposeAll()
301+
inline void destroyAll()
289302
{
290-
if (m_dispose_f && m_begin != invalid_iterator)
303+
uint32_t currentAddress = m_begin;
304+
while (currentAddress != invalid_iterator)
291305
{
292-
auto* begin = getBegin();
293-
auto* back = getBack();
294-
while (begin != back)
295-
{
296-
m_dispose_f(begin->data);
297-
begin = get(begin->next);
298-
}
299-
m_dispose_f(back->data);
306+
node_t* currentNode = get(currentAddress);
307+
uint32_t nextAddress = currentNode->next;
308+
if (m_dispose_f) m_dispose_f(currentNode->data);
309+
currentNode->~node_t();
310+
currentAddress = nextAddress;
300311
}
301312
}
302313

@@ -319,6 +330,8 @@ class DoublyLinkedList
319330
allocator_t m_allocator;
320331
address_allocator_t m_addressAllocator;
321332
void* m_reservedSpace;
333+
// In term of nodes
334+
size_t m_currentAllocationSize;
322335
node_t* m_array;
323336

324337
uint32_t m_cap;

0 commit comments

Comments
 (0)