Custom STL in place allocator crashed

[Allen]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL is
http://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q=InPlaceAlloc.h

#include <vector>
#include "InPlaceAlloc.h"

int main()
{
char * buffer = new char[1024];
typedef InPlaceAlloc<int> IPA;
std::vector<int, IPA> v(IPA(buffer, 1024));

// insert elements
// - causes reallocations
v.push_back(42);
v.push_back(56);
v.push_back(11);
v.push_back(22);
v.push_back(33);
v.push_back(44);

delete[] buffer;
buffer = NULL;
}

Run the program in MSVC8, it will crash.

Why? Please help me. Thank you.

Allen

 

[red floyd]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL is
http://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q=InPlaceAlloc.h

#include <vector>
#include "InPlaceAlloc.h"

int main()
{
char * buffer = new char[1024];
typedef InPlaceAlloc<int> IPA;
std::vector<int, IPA> v(IPA(buffer, 1024));

// insert elements
// - causes reallocations
v.push_back(42);
v.push_back(56);
v.push_back(11);
v.push_back(22);
v.push_back(33);
v.push_back(44);

delete[] buffer;
buffer = NULL;

At this point, v's destructor has yet to run.

 

[Allen]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q...

#include <vector>
#include "InPlaceAlloc.h"

int main()
{
char * buffer = new char[1024];
typedef InPlaceAlloc<int> IPA;
std::vector<int, IPA> v(IPA(buffer, 1024));

It crashs at above line.

// insert elements
// - causes reallocations
v.push_back(42);
v.push_back(56);
v.push_back(11);
v.push_back(22);
v.push_back(33);
v.push_back(44);

delete[] buffer;
buffer = NULL;

}

Run the program in MSVC8, it will crash.

Why? Please help me. Thank you.

Allen

 

[Thomas J. Gritzan]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q...

#include <vector>
#include "InPlaceAlloc.h"

int main()
{
char * buffer = new char[1024];

Avoid new[]. Avoid delete[]. Except buried down in a class or library.
You can use std::vector to allocate a buffer:

It crashs at above line.

It crashs, because InPlaceAlloc is broken. The copy constructor and the
assignment operator are defined but empty with a comment saying "do
nothing". That won't work.

 

[tni]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL is
http://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q=InPlaceAlloc.h

#include <vector>
#include "InPlaceAlloc.h"

int main()
{
char * buffer = new char[1024];
typedef InPlaceAlloc<int> IPA;
std::vector<int, IPA> v(IPA(buffer, 1024));

You do realize that you are using an int vector with a char sized
buffer, right?

It should be:
char * buffer = new char[1024 * sizeof(int)];

 

[zr]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q...
#include <vector>
#include "InPlaceAlloc.h"
int main()
{
char * buffer = new char[1024];

Avoid new[]. Avoid delete[]. Except buried down in a class or library.
You can use std::vector to allocate a buffer:

std::vector<char> buffer(1024);

Why is using a vector to allocate better than using new[] and delete
[]?

 

[Juha Nieminen]

Why is using a vector to allocate better than using new[] and delete
[]?

Because a std::vector is scoped, which means that when it goes out of
scope, it will delete the array automatically, thus lessening the
chances of a memory leak (especially inside functions which may have
surprising exit points).

Basically std::vector is an encapsulated way of dynamically allocating
an array.

When you write a 'new' and use the raw pointer, you always risk a
memory leak if you are not very careful.

 

[Thomas J. Gritzan]

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q...
#include <vector>
#include "InPlaceAlloc.h"
int main()
{
char * buffer = new char[1024];

Avoid new[]. Avoid delete[]. Except buried down in a class or library.
You can use std::vector to allocate a buffer:

std::vector<char> buffer(1024);

Why is using a vector to allocate better than using new[] and delete
[]?

In general, because it lessens the risk of memory leaks.
With new[], you have to delete[] in every possible code path, which can
be quite complicated in some functions, especially if you allocate
multiple arrays.
It is very hard to make it exception safe. Most people don't even try it
and ignore that issue. With std::vector or any other smart-pointer or
RAII construct, the objects will be destroyed even then.

In this special case, the buffer is used by the other std::vector and
its allocator. The code delete[]s the buffer to early while it is still
in use. But destructors run in reverse order to its constructor calls,
so if you use std::vector in this case, the buffer won't be deallocated
until the vector that uses this buffer is alive.

 

[James Kanze]

Allen schrieb:

There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HC lD5fLW7P8/InPlaceAlloc.h&q...
#include <vector>
#include "InPlaceAlloc.h"
int main()
{
char * buffer = new char[1024];
Avoid new[]. Avoid delete[]. Except buried down in a class or library.
You can use std::vector to allocate a buffer:
std::vector<char> buffer(1024);

Why is using a vector to allocate better than using new[] and delete
[]?

Orthodoxy.

In the usual case where the lifetime is scoped, it's the
simplest way to ensure exception safety, and definitly to be
prefered. Any time the array is to be passed as a parameter, it
acts like a real type, with value semantics and no special ad
hoc rules about converting to a pointer to the first element,
and it carries its size around with it as well. Most
implementations will also bounds check, which is really, really
rare for built in arrays. (I think I've only heard of one
compiler which bounds checks built-in arrays, and I don't know
if it is even still around.)

In 20 years of programming C++, I've never used an operator
new[]. (I do still use C style arrays in certain cases, but
never dynamically allocated.)

 

[James Kanze]

Allen schrieb:
There is a custom STL in place allocator in Google codes.
The InPlaceAlloc.h URL ishttp://www.google.com/codesearch/p?hl=en#HClD5fLW7P8/InPlaceAlloc.h&q...
#include <vector>
#include "InPlaceAlloc.h"
int main()
{
char * buffer = new char[1024];
Avoid new[]. Avoid delete[]. Except buried down in a class or library.
You can use std::vector to allocate a buffer:
std::vector<char> buffer(1024);

Why is using a vector to allocate better than using new[]
and delete []?

In general, because it lessens the risk of memory leaks. With
new[], you have to delete[] in every possible code path, which
can be quite complicated in some functions, especially if you
allocate multiple arrays. It is very hard to make it
exception safe. Most people don't even try it and ignore that
issue. With std::vector or any other smart-pointer or RAII
construct, the objects will be destroyed even then.

That's true, but not really relevant here. Similarly, a much
more important reason for using std::vector is that in any
decent implementation, the operator[] will be bounds checked,
and the type in general behaves like a normal type, with value
semantics. And that the type carries its dimensions along with
it.

None of which are really relevant here, because the user isn't
allocating an array, in the classical sense, he's allocating raw
memory. And the problem with new char[] is that it says the
wrong thing; it says that he is allocating an array of char. I
know, the language allows an array of char to be used as raw
memory. But that doesn't mean that you can't write code which
says what you mean. In this case, I'd call the :perator new
function directly, e.g.:

void* buffer = :perator new( quantityNeeded ) ;
// ...
:perator delete( buffer ) ;

(I'd also maintain the pointer as a void* where ever possible,
since this is the conventional type for a pointer to raw
memory.)

With regards to his original code:

1. I'd also ensure that the allocator took not only a pointer,
but also the size, and verify that constantly. (Part of his
problem, at least, is that the array he's allocating isn't
big enough.)

2. I'd define some memory management strategy. The simplest,
here, is just to use the Boehm collector; another solution
I've found useful in many cases is to pass an additional
boolean argument to the client, telling it whether to delete
in the destructor or not---since allocator must support
copy, this would require some tricky reference counting
here. Or... Just a guess, but it wouldn't surprise me if in
all of the actual use cases for this class, the memory
doesn't really have to be deleted anyway. (But if he'd
posted code without the delete, a lot of people would have
screamed about that.)

 

[James Kanze]

On 2008-12-31 09:37:54 -0500, zr <[email protected]> said:
On Dec 31, 4:20 am, "Thomas J. Gritzan" <[email protected]>
wrote:
Allen schrieb:
#include <vector>
#include "InPlaceAlloc.h"
int main()
{
char * buffer = new char[1024];
Avoid new[]. Avoid delete[]. Except buried down in a
class or library. You can use std::vector to allocate a
buffer:
std::vector<char> buffer(1024);
Why is using a vector to allocate better than using new[]
and delete[]?
Orthodoxy.

In the usual case where the lifetime is scoped, it's the
simplest way to ensure exception safety, and definitly to be
prefered.

In the example code, it had nothing to do with the problem.

Yes. I only saw the complete example later; in this particular
case (allocating raw memory), I wouldn't recommend std::vector,
at least not systematically. But in this case, I'd prefer that
the statement make it clear that I was allocating raw memory,
i.e.:
void* buffer = :perator new( requiredSize ) ;
It's just a coding convention, but I find it a clear way to say:
attention: this is raw memory, and not an array of characters.

But I guess there's a difference in viewpoint here. I think
it's important to try to give a clear answer to the question
that was asked. Others like to pontificate on how the sample
code didn't meet their coding guidelines, with so much
lecturing that the answer to the question gets lost. Oddly
enough, the original problem no longer appears in the quoted
text.

Exactly. Which is why I reacted to your response: in almost all
cases, std::vector is preferable. This just happens to be an
exception.