Bug 80 - valgrind error when finalizing PacketMetadata
valgrind error when finalizing PacketMetadata
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: core
pre-release
PC Linux
: P1 normal
Assigned To: ns-bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-27 12:56 UTC by Gustavo J. A. M. Carneiro
Modified: 2007-10-08 12:44 UTC (History)
0 users

See Also:


Attachments
possible patch (2.28 KB, patch)
2007-09-27 13:24 UTC, Gustavo J. A. M. Carneiro
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo J. A. M. Carneiro 2007-09-27 12:56:00 UTC
A simulation runs flawlessly, but in the end when main is exiting:

==14231== Invalid write of size 8
==14231==    at 0x50858B8: __gnu_cxx::new_allocator<ns3::PacketMetadata::Data*>::construct(ns3::PacketMetadata::Data**, ns3::PacketMetadata::Data* const&) (new_allocator.h:104)
==14231==    by 0x5086AD2: std::vector<ns3::PacketMetadata::Data*, std::allocator<ns3::PacketMetadata::Data*> >::push_back(ns3::PacketMetadata::Data* const&) (stl_vector.h:606)
==14231==    by 0x5083892: ns3::PacketMetadata::Recycle(ns3::PacketMetadata::Data*) (packet-metadata.cc:655)
==14231==    by 0x40959E: ns3::PacketMetadata::~PacketMetadata() (packet-metadata.h:340)
==14231==    by 0x409681: ns3::Packet::~Packet() (packet.h:75)
==14231==    by 0x50F9725: ns3::UdpSocketTest::~UdpSocketTest() (udp-socket.cc:352)
==14231==    by 0x50F466B: __tcf_1 (udp-socket.cc:459)
==14231==    by 0x5BE983F: __cxa_finalize (in /lib/libc-2.6.1.so)
==14231==    by 0x4FF4742: (within /home/gjc/projects/ns-3/ns-3-wmn/build/debug/libns3.so)
==14231==    by 0x513B880: (within /home/gjc/projects/ns-3/ns-3-wmn/build/debug/libns3.so)
==14231==    by 0x5BE94FF: exit (in /lib/libc-2.6.1.so)
==14231==    by 0x5BD2B4A: (below main) (in /lib/libc-2.6.1.so)
==14231==  Address 0x5f33758 is 8 bytes inside a block of size 64 free'd
==14231==    at 0x4C2162D: operator delete(void*) (vg_replace_malloc.c:336)
==14231==    by 0x50866EE: __gnu_cxx::new_allocator<ns3::PacketMetadata::Data*>::deallocate(ns3::PacketMetadata::Data**, unsigned long) (new_allocator.h:94)
==14231==    by 0x5086720: std::_Vector_base<ns3::PacketMetadata::Data*, std::allocator<ns3::PacketMetadata::Data*> >::_M_deallocate(ns3::PacketMetadata::Data**, unsigned long) (stl_vector.h:133)
==14231==    by 0x5086D97: std::_Vector_base<ns3::PacketMetadata::Data*, std::allocator<ns3::PacketMetadata::Data*> >::~_Vector_base() (stl_vector.h:119)
==14231==    by 0x5086DEE: std::vector<ns3::PacketMetadata::Data*, std::allocator<ns3::PacketMetadata::Data*> >::~vector() (stl_vector.h:272)
==14231==    by 0x5083905: ns3::PacketMetadata::DataFreeList::~DataFreeList() (packet-metadata.cc:46)
==14231==    by 0x508394D: __tcf_1 (packet-metadata.cc:37)
==14231==    by 0x5BE983F: __cxa_finalize (in /lib/libc-2.6.1.so)
==14231==    by 0x4FF4742: (within /home/gjc/projects/ns-3/ns-3-wmn/build/debug/libns3.so)
==14231==    by 0x513B880: (within /home/gjc/projects/ns-3/ns-3-wmn/build/debug/libns3.so)
==14231==    by 0x5BE94FF: exit (in /lib/libc-2.6.1.so)
==14231==    by 0x5BD2B4A: (below main) (in /lib/libc-2.6.1.so)


I am not very familiar with the metadata code, but it looks like the system was caught in a state where there exists a packet using a data segment that also belongs to the metadata "free list".  The "free list" is destroyed and frees the data segment; subsequently the packet is also destroyed and also frees the metadata which tries to free the same data a second time.
Comment 1 Gustavo J. A. M. Carneiro 2007-09-27 13:12:44 UTC
OK, I think I understand the problem better now...

==14271==    by 0x5086AD2: std::vector<ns3::PacketMetadata::Data*, std::allocator<ns3::PacketMetadata::Data*> >::push_back(ns3::PacketMetadata::Data* const&) (stl_vector.h:606)
==14271==    by 0x5083892: ns3::PacketMetadata::Recycle(ns3::PacketMetadata::Data*) (packet-metadata.cc:655)
==14271==    by 0x40959E: ns3::PacketMetadata::~PacketMetadata() (packet-metadata.h:340)
==14271==    by 0x409681: ns3::Packet::~Packet() (packet.h:75)


When the packet is destroyed, it tries to add the metadata chunk back to the free list.  But the free list vector has already been destroyed at that point...

Now, any opinions on which is the best solution:
 1. Add a boolean global variable telling when the free list has already been destroyed;
 2. Make the "static DataFreeList m_freeList;" member of PacketMetadata a pointer, allocate it with new, never deallocate.
 3. Other option?
Comment 2 Gustavo J. A. M. Carneiro 2007-09-27 13:24:22 UTC
Created attachment 70 [details]
possible patch
Comment 3 Mathieu Lacage 2007-10-08 02:50:14 UTC
(In reply to comment #1)
> OK, I think I understand the problem better now...
> 
> ==14271==    by 0x5086AD2: std::vector<ns3::PacketMetadata::Data*,
> std::allocator<ns3::PacketMetadata::Data*>
> >::push_back(ns3::PacketMetadata::Data* const&) (stl_vector.h:606)
> ==14271==    by 0x5083892:
> ns3::PacketMetadata::Recycle(ns3::PacketMetadata::Data*)
> (packet-metadata.cc:655)
> ==14271==    by 0x40959E: ns3::PacketMetadata::~PacketMetadata()
> (packet-metadata.h:340)
> ==14271==    by 0x409681: ns3::Packet::~Packet() (packet.h:75)
> 
> 
> When the packet is destroyed, it tries to add the metadata chunk back to the
> free list.  But the free list vector has already been destroyed at that
> point...

ok.

> 
> Now, any opinions on which is the best solution:
>  1. Add a boolean global variable telling when the free list has already been
> destroyed;

That might work but it leaves me a bad taste in the mouth afterwards.

>  2. Make the "static DataFreeList m_freeList;" member of PacketMetadata a
> pointer, allocate it with new, never deallocate.

This is what was implemented before: it just triggers memory leak reports by valgrind.

>  3. Other option?

I can't think of any better option.

Potentially, I think that option 1) is the most useful because it avoids memory leak reports and eliminates this problem.
Comment 4 Gustavo J. A. M. Carneiro 2007-10-08 12:44:53 UTC
OK, fixed with boolean flag (m_enable reused for this).