Skip to content

Commit

Permalink
Improve memory manager for MSVC exceptions in c-tor args
Browse files Browse the repository at this point in the history
MSVC calls allocator first before evaluating arguments for
the actual object constructor. Since the evaluation phase
can quite easily throw an error (like in `parse_list`), the
memory manager is left in an undefined state. The object is
already tracked but not initialized and we get an error if
we try to `delete` the object (we should use free instead).

Added macro to be used instead of overloaded new operator!
  • Loading branch information
mgreter committed Aug 24, 2015
1 parent 8fbaaf7 commit e24865f
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
27 changes: 19 additions & 8 deletions src/memory_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Sass {
Memory_Manager<T>::~Memory_Manager()
{
// release memory for all controlled nodes
// avoid calling erase for every single node
// avoid calling erase for every single node
for (size_t i = 0, S = nodes.size(); i < S; ++i) {
deallocate(nodes[i]);
}
Expand All @@ -26,11 +26,10 @@ namespace Sass {
}

template <typename T>
T* Memory_Manager<T>::operator()(T* np)
T* Memory_Manager<T>::add(T* np)
{
// add to pool
nodes.push_back(np);
// return resource
void* heap = (char*)np - 1;
*((char*)((void*)heap)) = 1;
return np;
}

Expand All @@ -44,13 +43,25 @@ namespace Sass {
template <typename T>
T* Memory_Manager<T>::allocate(size_t size)
{
return static_cast<T*>(operator new(size));
// need additional memory for status header
void* heap = malloc(sizeof(char) + size);
// init internal status to zero
*(static_cast<char*>(heap)) = 0;
// the object lives on char further
void* object = static_cast<char*>(heap) + 1;
// add the memory under our management
nodes.push_back(static_cast<T*>(object));
// cast object to its final type
return static_cast<T*>(object);
}

template <typename T>
void Memory_Manager<T>::deallocate(T* np)
{
delete np;
void* object = static_cast<void*>(np);
char* heap = static_cast<char*>(object) - 1;
if (heap[0]) np->~T();
free(heap);
}

template <typename T>
Expand All @@ -67,7 +78,7 @@ namespace Sass {
// remove from pool
remove(np);
// release memory
delete np;
deallocate(np);
}

// compile implementation for AST_Node
Expand Down
12 changes: 10 additions & 2 deletions src/memory_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,25 @@ namespace Sass {
void deallocate(T* np);
void remove(T* np);
void destroy(T* np);
T* operator()(T* np);
T* add(T* np);

};
}

template <typename T>
inline void* operator new(size_t size, Sass::Memory_Manager<T>& mem)
{ return mem(mem.allocate(size)); }
{ return mem.add(mem.allocate(size)); }

template <typename T>
inline void operator delete(void *np, Sass::Memory_Manager<T>& mem)
{ mem.destroy(reinterpret_cast<T*>(np)); }

///////////////////////////////////////////////////////////////////////////////
// Use macros for the allocation task, since overloading operator `new`
// has been proven to be flaky under certain compilers (see comment below).
///////////////////////////////////////////////////////////////////////////////

#define SASS_MEMORY_NEW(mgr, Class, ...) \
(static_cast<Class*>(mgr.add(new (mgr.allocate(sizeof(Class))) Class(__VA_ARGS__)))) \

#endif

0 comments on commit e24865f

Please sign in to comment.