Re: Defining placement new[] and delete[]
On Dec 24, 11:07 am, James Kanze <james.ka...@gmail.com> wrote:
On Dec 23, 9:56 pm, zr <zvir...@gmail.com> wrote:
I tried adding my own implementations of:
void* operator new[](size_t nbytes),
void* operator new[](size_t nbytes, Pool& pool) and
void operator delete[](void* p)
{{start of code}}
#include <iostream>
class Pool {
public:
Pool(size_t _poolSize)
{
#if _DEBUG
std::cout << __FUNCTION__ << " Pool add=
ress = " << this << "\n";
#endif
nextAvail = start = (char*)malloc(_=
poolSize);
if (NULL==start)
{
throw std::bad_alloc();
}
end = start + _poolSize;
}
~Pool() {
free(start);
}
void* alloc(size_t nbytes) {
if (checkAllocations)
{
if (nbytes > avail())
{
throw s=
td::bad_alloc();
}
}
char* pos = nextAvail;
nextAvail += nbytes;
return pos;
}
void dealloc(void* p) {
// delloc currenty does nothing
}
size_t avail() const {
return end-nextAvail;
}
size_t allocated() const {
return nextAvail-start;
}
protected:
char* start;
char* end;
char* nextAvail;
static const bool checkAllocations=true;
};
void* operator new(size_t nbytes)
{
if (nbytes == 0)
nbytes = 1; =
// so all alloc's get a distinct
address
void* ans = malloc(nbytes + 4); // overallocate by=
4 bytes
*(Pool**)ans = NULL; // use N=
ULL in the global new
return (char*)ans + 4; // don't let=
users see the Pool*
}
Don't use 4 here. First, it's not guaranteed to be the right
size (for that, you'd want "sizeof(Pool*)"), second, it can
result in bad alignment in the returned address. (On my
machine, a Sun Sparc, your code will core dump if the new'ed
structure contains a long long or a double.) Handling the
alignment portably can be tricky, but for any given machine, you
can use a union with the type which requires the strictest
alignment (usually double or long double), e.g.:
union BlockHeader
{
Pool* source ;
MaxAlign dummyForAlignment ;
} ;
(where MaxAlign is a machine dependent typedef... or a union of
all plausible types.) And you set the pointer with
((BlockHeader*)ans)->source = ... ;
(The way I usually do this is:
void*
operator new(
size_t byteCount )
{
BlockHeader* result
= (BlockHeader*)malloc( byteCount + sizeof
( BlockHeader ) ) ;
result->source = NULL ;
return result + 1 ;
}
. Let the compiler worry about how many bytes in BlockHeader.)
Also, since you're already adding 4, you don't need to convert 0
into 1.
Point Taken. As mentioned in the OP, i used the example from Marshal
Cline's FAQ, and wanted to make minimal changes to make things work.
Besides, i ran this test on WIN32, so the pointer size and alignment
were good enough for it run with no crash.
void* operator new[](size_t nbytes)
{
if (nbytes == 0)
nbytes = 1; =
// so all alloc's get a distinct
address
void* ans = malloc(nbytes + 4); // overallocate by=
4 bytes
*(Pool**)ans = NULL; // use N=
ULL in the global new
void* res = (char*)ans + 4;
#if _DEBUG
std::cout << __FUNCTION__ << "returned=" << res<< "\n=
";
#endif
return res; // don't let users see =
the Pool*
}
Much simpler solution:
void*
operator new[](
size_t byteCount )
{
return operator new( byteCount ) ;
}
Or simply do nothing; this is the required behavior of the
version in the standard library.
void* operator new(size_t nbytes, Pool& pool)
{
if (nbytes == 0)
nbytes = 1; =
// so all alloc's get a distinct
address
void* ans = pool.alloc(nbytes + 4); // overallocate b=
y 4 bytes
*(Pool**)ans = &pool; // put t=
he Pool* here
void* res = (char*)ans + 4;
#if _DEBUG
std::cout << __FUNCTION__ << "returned=" << res<< "\n=
";
#endif
return res; // don't let users see =
the Pool*
}
void* operator new[](size_t nbytes, Pool& pool)
{
if (nbytes == 0)
nbytes = 1; =
// so all alloc's get a distinct
address
void* ans = pool.alloc(nbytes + 4); // overallocate b=
y 4 bytes
*(Pool**)ans = &pool; // put t=
he Pool* here
void* res = (char*)ans + 4;
#if _DEBUG
std::cout << __FUNCTION__ << " returned=" << res<< "\=
n";
#endif
return res; // don't let users see =
the Pool*
}
As above. Including the fact that the array version can
delegate to the non-array version.
void operator delete(void* p)
{
#if _DEBUG
std::cout << __FUNCTION__ << " freed=" << p;
#endif
if (p != NULL) {
p = (char*)p - 4; =
// back off to the Pool*
Pool* pool = *(Pool**)p;
if (pool == NULL)
{
free(p); =
// note: 4 bytes left of the original
p
#if _DEBUG
std::cout << " from gen=
eral pool\n";
#endif
}
else
{
pool->dealloc(p); =
// note: 4 bytes left of the original
p
#if _DEBUG
std::cout << " from poo=
l " << pool << "\n";
#endif
}
}
}
Again:
void
operator delete(
void* userPointer )
{
BlockHeader* block
= ((BlockHeader*)userPointer) - 1 ;
if ( block->source == NULL ) {
free( block ) ;
} else {
block->source->dealloc( block ) ;
}
}
Let the compiler keep track of the different sizes.
void operator delete[](void* p)
{
#if _DEBUG
std::cout << __FUNCTION__ << " freed=" << p;
#endif
if (p != NULL) {
p = (char*)p - 4; =
// back off to the Pool*
Pool* pool = *(Pool**)p;
if (pool == NULL)
{
free(p); =
// note: 4 bytes left of the original
p
#if _DEBUG
std::cout << " from gen=
eral pool\n";
#endif
}
else
{
pool->dealloc(p); =
// note: 4 bytes left of the original
p
#if _DEBUG
std::cout << " from poo=
l " << pool << "\n";
#endif
}
}
}
And delete in delete as well.
This is great feedback. Thanks.
class A {
public:
A()
{
#if _DEBUG
std::cout << __FUNCTION__ << "\n";
#endif
}
~A()
{
#if _DEBUG
std::cout << __FUNCTION__ << "\n";
#endif
}
};
// Allocates an array using placement new[]
// and then throw an exception
class Bomb {
public:
Bomb(Pool& pool) {
#if _DEBUG
std::cout << __FUNCTION__ << "\n";
#endif
a = new(pool) A[4];
std::cout << "Now comes the exception..=
..\n";
throw std::bad_alloc();
}
~Bomb() {
#if _DEBUG
std::cout << __FUNCTION__ << "\n";
#endif
delete [] a;
}
private:
A* a;
};
int _tmain(int argc, _TCHAR* argv[])
{
try {
Pool pool(1024*1024);
Bomb boom(pool);
} catch (std::bad_alloc& e)
{
std::cout << "A::A~ should have appeare=
d above\n";
std::cout << e.what() << std::endl;
}
return 0;
}
{{end of code}}
The compiler gives the following warning:
warning C4291: 'void *operator new[](size_t,Pool &)' : no matching
operator delete found; memory will not be freed if initialization
throws an exception
d:\bufferpool\bufferpool.cpp(93) : see declaration of 'operator new[]'
and sure enough the allocated array was not auto-magically
destroyed as seen in the output:
Pool::Pool Pool address = 0012FF48
Bomb::Bomb
operator new[] returned=00420044
A::A
A::A
A::A
A::A
Now comes the exception...
A::A~ should have appeared above
No it shouldn't have. Objects constructed with a new should
only be destructed if the code calls delete. Once you've
successfully completed the new, the compiler never calls delete
on it.
Whoops. I totally misunderstood that. Thanks for pointing that out.
There is a case where you're missing something, however. If the
constructor of A throws, e.g.:
static int trigger = 0 ;
class A
{
public:
A()
{
std::cout << "A::A" << std::endl ;
if ( trigger != 0 ) {
-- trigger ;
if ( trigger == 0 ) {
throw someException ;
}
}
}
~A()
{
std::cout << "A::~A" << std::endl ;
}
} ;
int
main()
{
try {
trigger = 3 ;
A* p = new A[ 4 ] ;
} catch ( ... ) {
}
}
should output:
A::A
A::A
A::A
A::~A
A::~A
Ok, it does output that.
With your placement (Pool*) operator new, however, it will leak
memory. (You can instrument this in your Pool class, keeping
track of how many blocks are actually allocated.) To avoid
this, you need an operator delete( void*, Pool& ) and an
operator delete[]( void*, Pool& ). Note that these will never
be called in the case of a normal delete; only if an operator
new terminates because of an exception from a constructor.
But the whole point of this exercise was to define a smart delete
operator such that the client code does not need to associate the
deleted pointer with the memory pool that allocated it.
So, is it possible to fix the above code to auto-magically
destroy the array after the execption occurs?
In the case of the above code, it's the responsability of the
client (Bomb) to ensure that the delete is properly called.
The usual way of doing this is to change the pointer in Bomb to
a smart pointer, whose destructor *will* be called if an
exception is thrown in the constructor of Bomb (since it will be
a completely constructed sub-object).
What about using a try-catch block in Bomb::Bomb() ?