Bug 1422 - valgrind error in ipv6-fragmentation test suite
valgrind error in ipv6-fragmentation test suite
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: ipv6
ns-3-dev
All All
: P5 normal
Assigned To: Tommaso Pecorella
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-04 05:22 UTC by Nicola Baldo
Modified: 2012-05-23 01:19 UTC (History)
2 users (show)

See Also:


Attachments
output of valgrind --leak-check=full (4.99 KB, text/plain)
2012-05-04 05:22 UTC, Nicola Baldo
Details
Fix the valgring error (5.43 KB, patch)
2012-05-22 15:53 UTC, Tommaso Pecorella
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nicola Baldo 2012-05-04 05:22:47 UTC
Created attachment 1395 [details]
output of valgrind --leak-check=full

using ns-3-dev changeset 3eda7174ac52

$ ./test.py --grind -s ipv6-fragmentation
[snip]
VALGR: TestSuite ipv6-fragmentation
0 of 1 tests passed (0 passed, 0 skipped, 0 failed, 0 crashed, 1 valgrind errors)

I got this on a ubuntu 11.10 64bit machine. 
I am also attaching a log produced with this command:
./waf --run test-runner --command="valgrind --leak-check=full %s --suite=ipv6-fragmentation"
Comment 1 Tom Henderson 2012-05-04 11:51:35 UTC
(In reply to comment #0)
> Created attachment 1395 [details]
> output of valgrind --leak-check=full
> 
> using ns-3-dev changeset 3eda7174ac52
> 
> $ ./test.py --grind -s ipv6-fragmentation
> [snip]
> VALGR: TestSuite ipv6-fragmentation
> 0 of 1 tests passed (0 passed, 0 skipped, 0 failed, 0 crashed, 1 valgrind
> errors)
> 
> I got this on a ubuntu 11.10 64bit machine. 
> I am also attaching a log produced with this command:
> ./waf --run test-runner --command="valgrind --leak-check=full %s
> --suite=ipv6-fragmentation"

This has been around for over a month, and I've been poking at it while I get time-- the fix has not been obvious.  I will keep at it.
Comment 2 Tommaso Pecorella 2012-05-21 18:32:39 UTC
I found the problem. It's not wise to use a Ptr<> to a SimpleRefCount as an argument to a Simulator::Schedule.

The solution is not that simple, tho, as it means to change how the fragment lost timeout is handled. By luck the IPv4 code is better at this (the event is handled by the fragments and not by the outer class. Sorry if all this is kinda obscure but I'm almost sleeping.

Tomorrow morning I'll make a patch.

T.

PS: and that's why when I copied the IPv6 fragment handling to make IPv4 one I used a different way to handle the timeout. The IPv6 one didn't felt "right". And I was right at not liking it.
Comment 3 Tommaso Pecorella 2012-05-22 15:53:26 UTC
Created attachment 1400 [details]
Fix the valgring error

As I tought. It's not wise to launch an event with a SimpleRefCount Ptr as an argument (or an Object either).

Also in this patch:
1) remove two unused and obscure mutable member variables from Fragments (both IPv4 and IPv6) and
2) changed the name of an inner member variable whose name was the same as an outer one

Number 1 is a remnant of extremely old SimpleRefCount implementation (probably),
Number 2 is cosmetic but it can save some brain cells to go fishin' while browsing the code.

Tested on a private Fedora release 16 (Verne). Jenkins seems to have some VMs down. Need a confirm.
Comment 4 Tom Henderson 2012-05-23 01:19:30 UTC
(In reply to comment #3)
> Created attachment 1400 [details]
> Fix the valgring error
> 
> As I tought. It's not wise to launch an event with a SimpleRefCount Ptr as an
> argument (or an Object either).
> 
> Also in this patch:
> 1) remove two unused and obscure mutable member variables from Fragments (both
> IPv4 and IPv6) and
> 2) changed the name of an inner member variable whose name was the same as an
> outer one
> 
> Number 1 is a remnant of extremely old SimpleRefCount implementation
> (probably),
> Number 2 is cosmetic but it can save some brain cells to go fishin' while
> browsing the code.
> 
> Tested on a private Fedora release 16 (Verne). Jenkins seems to have some VMs
> down. Need a confirm.

pushed as changeset 9c59d55abcce after spot testing-- thanks!