Bug 80

Summary:	valgrind error when finalizing PacketMetadata
Product:	ns-3	Reporter:	Gustavo J. A. M. Carneiro <gjcarneiro>
Component:	core	Assignee:	ns-bugs <ns-bugs>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P1
Version:	pre-release
Hardware:	PC
OS:	Linux
Attachments:	possible patch

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).