bug in map? 
Author Message
 bug in map?

If you instantiate a map AND insert at least one value, the map allocates
memory three times: Twice for the map itself and one for the inserted value.
The crucial point is, the constructor is never called for the first two
allocations, but the destructor is always called!
Thus, the following code chrashes:

struct test
{
 char *m_p;

 test()
 {
  m_p = new char;
 }
 ~test()
 {
  delete m_p;
 }

Quote:
};

void test()
{
 std::map<int, test> myMap;
 test s;
 myMap.insert(std::map<int, test>::value_type(0,s));

Quote:
}

Any ideas?
Klaus


Sun, 16 May 2004 01:18:23 GMT  
 bug in map?

Quote:

> If you instantiate a map AND insert at least one value, the map allocates
> memory three times: Twice for the map itself and one for the inserted value.
> The crucial point is, the constructor is never called for the first two
> allocations, but the destructor is always called!

Doesn't do that when I test it. The constructor is called once and the
destructor is called once, as should be the case.

Quote:
> Any ideas?

Check your work.

P.J. Plauger
Dinkumware, Ltd.
http://www.dinkumware.com



Sun, 16 May 2004 01:39:32 GMT  
 bug in map?


Quote:
> If you instantiate a map AND insert at least one value, the map allocates
> memory three times: Twice for the map itself and one for the inserted
value.
> The crucial point is, the constructor is never called for the first two
> allocations, but the destructor is always called!

Your code is wrong. Classic C++ problem.

If you allocate memory in the constructor and destroy it in the destructor,
then your class requires a copy constructor and an assignment operator
(which needs to also handle self-assignment properly) to handle this memory
correctly.
If you don't provide one, the compiler will generate both for you and they
will bitwise copy the pointer in question - not what you want !!!
Since many parts of the STL call copy constructors underneath (including
map), that is why you have the problem.

Here is an example which illustrates the problem (based on your code)

void foobar()
{
    test    t1;                // t1.m_p points to a char
    test    t2 = t1;        // copy constructor called, t2.m_p points to the
same char, problem coming up...

    // t2 's destructor called, delete on pointer called
    // t1 's destructor called, delete on same pointer called, ERROR,
ILLEGAL !!!

Quote:
}
> Any ideas?

Fix your code. There is a thing called the "rule of 3", which states that if
you need to do something "special" in the destructor (like deleting memory,
freeing some resource), then you will need a copy constructor and an
assignment operator.

If you class has either (i) simple data types like int, short,double or (ii)
string,vector etc which can take care of themselves, then you don't need a
copy constructor or assignment operator as the compiler-generated ones are
fine.

Hope this helps

BTW, don't be so quick to call something a bug, speaking for myself, 90% of
the time when something is wrong, it is my code :-)

Stephen Howe



Sun, 16 May 2004 01:57:39 GMT  
 bug in map?

Quote:
> > If you instantiate a map AND insert at least one value, the map
allocates
> > memory three times: Twice for the map itself and one for the inserted
value.
> > The crucial point is, the constructor is never called for the first two
> > allocations, but the destructor is always called!

> Doesn't do that when I test it. The constructor is called once and the
> destructor is called once, as should be the case.

I use SP4 with shipped dinkumware stl and the patches from dinkumware.
If you run the code until the instantiation of the map, set a breakpoint to
file xtree, line 580:
  {_Nodeptr _S = (_Nodeptr)allocator._Charalloc(

You will encounter two calls to _Charalloc, which forwards the call to
afxmem.cpp, line 316
void* __cdecl operator new(size_t nSize)

During the instantiation of the struct test (variable s), there is a call to
the constructor, of course.

During the insert, you will encounter again one call to _Charalloc and one
call to the destructor of the value type (the struct test in my example). If
you don't skip the free call, you get the first Access Violation, because
memory should be free'd which was never allocated!

While the function tidies up everything, you will encounter first the
destructor call for the struct test of variable s, this is ok.
After that, you will get the third call to the destructor of test. Again,
for this object, the constructor wasn't called! Thus, skip the delete
instruction.

Finally, you will encounter three calls to xtree, line 591:
  {allocator.deallocate(_S, 1); }

In summary, I get one constructor call and three destructor calls!

This sounds strange to me!???

Klaus



Sun, 16 May 2004 04:59:24 GMT  
 bug in map?

Quote:
> > If you instantiate a map AND insert at least one value, the map
allocates
> > memory three times: Twice for the map itself and one for the inserted
> value.
> > The crucial point is, the constructor is never called for the first two
> > allocations, but the destructor is always called!

> Your code is wrong. Classic C++ problem.

> If you allocate memory in the constructor and destroy it in the
destructor,
> then your class requires a copy constructor and an assignment operator
> (which needs to also handle self-assignment properly) to handle this
memory
> correctly.

You are right. But I removed this code for simplicity. If you add such copy
constructor code, you have still the same problem: Only one call to the
constructor, but three calls to the destructor. See my detailled answer to
P.J. Plauger.

Here is the code with the copy constructor and assignment operator:
struct test
{
 char *m_p;

 test()
 {
  m_p = new char;
 }
 test(const test & other)
 {
  *this = other;
 }
 test& operator = (const test & other)
 {
  if (this != &other)
  {
   // nothing to do, I'm not interested in the values pointed by m_p! That's
legal!
  }
  return *this;
 }
 ~test()
 {
  delete m_p;
 }

Quote:
};

void test()
{
 std::map<int, test> myMap;
 test s;
 myMap.insert(std::map<int, test>::value_type(0,s));

Quote:
}
> BTW, don't be so quick to call something a bug, speaking for myself, 90%
of
> the time when something is wrong, it is my code :-)

I agree and this is the reason for the question mark in the title.
Thanks for your help.

Klaus



Sun, 16 May 2004 05:05:37 GMT  
 bug in map?
Your copy constructor leaves m_p uninitialized, containing garbage.
--
With best wishes,
    Igor Tandetnik

"For every complex problem, there is a solution that is simple, neat, and
wrong." H.L. Mencken


Quote:
> > > If you instantiate a map AND insert at least one value, the map
> allocates
> > > memory three times: Twice for the map itself and one for the inserted
> > value.
> > > The crucial point is, the constructor is never called for the first
two
> > > allocations, but the destructor is always called!

> > Your code is wrong. Classic C++ problem.

> > If you allocate memory in the constructor and destroy it in the
> destructor,
> > then your class requires a copy constructor and an assignment operator
> > (which needs to also handle self-assignment properly) to handle this
> memory
> > correctly.

> You are right. But I removed this code for simplicity. If you add such
copy
> constructor code, you have still the same problem: Only one call to the
> constructor, but three calls to the destructor. See my detailled answer to
> P.J. Plauger.

> Here is the code with the copy constructor and assignment operator:
> struct test
> {
>  char *m_p;

>  test()
>  {
>   m_p = new char;
>  }
>  test(const test & other)
>  {
>   *this = other;
>  }
>  test& operator = (const test & other)
>  {
>   if (this != &other)
>   {
>    // nothing to do, I'm not interested in the values pointed by m_p!
That's
> legal!
>   }
>   return *this;
>  }
>  ~test()
>  {
>   delete m_p;
>  }
> };

> void test()
> {
>  std::map<int, test> myMap;
>  test s;
>  myMap.insert(std::map<int, test>::value_type(0,s));
> }

> > BTW, don't be so quick to call something a bug, speaking for myself, 90%
> of
> > the time when something is wrong, it is my code :-)

> I agree and this is the reason for the question mark in the title.
> Thanks for your help.

> Klaus



Sun, 16 May 2004 05:11:31 GMT  
 bug in map?

Quote:
> Your copy constructor leaves m_p uninitialized, containing garbage.

Right. Instead of calling the constructor the copy constructor is called. I
didn't see that. Thanks.

Klaus



Sun, 16 May 2004 18:15:27 GMT  
 bug in map?

Quote:
> You are right. But I removed this code for simplicity.

Since this _crucial_ for your class to work at all and since you were
claiming that here maybe a bug in map, omitting this code was not sensible
:-|

This copy constructor does not handle m_p correctly and so it won't work.
Change

test(const test & other)
{
    *this = other;

Quote:
}

to

test(const test & other)
{
    m_p = new char;
    *m_p = other->m_p; // copy of only character

Quote:
}

and I think you will find that your breakpoints on the allocators memory
handling will balance.

Finally, if you put some debug messages in the constructor, copy constructor
and destructor, I think you will find that

Number of Constructor calls + Number of Copy Constructor calls == Number of
Destructor calls.

FWIW, I tested map some time ago to see what effect different methods of
insert did. Here is a copy of my post:

(IMPORTANT!!! :  Because I am showing _just_ what happens on an insert, the
equation below does not balance, it is out by 1 destructor, the one that
gets called when the map is detroyed).

Recently I tested map inserts using a test class as the 2nd parameter which
had calls to cout  so that I could see what was going on.

The results were interesting.

I tested 4 ways of inserting an element into a map

(i) use map.insert(make_pair(key,value))
(ii) use map.insert(map::value_type(key,value))
(iii) use map.insert(pair(key,value))
(iv use map[key] = value;

I compiled and ran 4 programs. Results were per insert

      ctor   cctor    dtor
(i)       -      3        2
(ii)      -      2        1
(iii)     -      3        2
(iv)     1      2        2

So I see that value_type is most efficient.

At this point I wondering why I would ever use make_pair() or pair(). I know
they have uses other than just map's.



Sun, 16 May 2004 20:56:00 GMT  
 bug in map?

Quote:
> In summary, I get one constructor call and three destructor calls!

I am not convinced. I tested map sometime ago and results were different
from what you say.
Please could you put some messages in the constructor, copy constructor and
destructor.
I think you will find that they balance

Stephen Howe



Sun, 16 May 2004 20:57:26 GMT  
 
 [ 9 post ] 

 Relevant Pages 

1. VC5.0 - STL bug in MAP!

2. a bug in map< >.end()

3. bug in map<>.erase

4. WTL bug in Property Sheet message maps?

5. STL map::find() bug?

6. map::operator[] bug in VC5.3 !

7. VC5.0 STL map bug - Alpha only!

8. STL map Bug?

9. Bugs in std::map?

10. Maps+CRITICAL_SECTION+XTREE bug fix

11. STL map::find() bug?

12. Chicago Map's Precision Mapping OCX

 

 
Powered by phpBB® Forum Software