BIND 10 #2669: Valgrind issues

[BIND 10 Development]

#2669: Valgrind issues
-------------------------------------+-------------------------------------
            Reporter:  jreed         |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  Unclassified  |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130219
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:6 jelte]:

 > > > I got rid of most of the fork()-related issues by adding --child-
 silent-after-fork=yes (technically this could result in missed issues in
 forking unit tests, but currently it is reporting leaks from stack
 allocations...)
 > >
 > > Could you elaborate a bit more?  What does "leaks from stack
 > > allocations" mean?  Specifically what kind of error was reported for
 > > which part of the code?  Isn't there any false negatives by
 > > suppressing this? (and if there is, are you suggesting suppressing
 > > others at the cost of missing them is more beneficial?)

 > Here's the first that gets hit on my system:
 [...]
 > (note, perhaps it is slightly more clear that it is about the
 std::string creation if you change the test to first create a string
 variable and pass that to the constructor of InterProcessSyncLocker)
 >
 > From what I can tell, it's not even so much the fork() itself, it's the
 call to exit() that makes it not free all. Perhaps we can get around that
 by adding an ExitPlease exception that is caught in the main() of our unit
 tests.
 >
 > (simply returning instead of exit appears to make other tests fail, and
 making a local try/catch that exists has the same problem since then the
 exception itself isn't freed)
 >
 > If you have any other ideas, or if i missed something obvious, please
 let me know :)

 This seems to me to be another reason why it's a bad idea to try to
 invent such low level wheel ourselves:-)  That aside, in this
 particular case, can't we delay defining of `sync` and `locker` after
 fork and have the child wait on the pipe until the parent acquires the
 lock?  But, even if it works, other cases would have to be handled
 case-by-case basis, so it may not help much.

 Since I don't know how many other cases we have and how difficult they
 are to work around, it's hard to say whether the global suppression is
 a reasonable choice.  If the cases are not so many (and assuming the
 fork generally happens in unit tests), another possibility is to skip
 these test cases via gtest filter when running valgrind on buildbots.
 That way, we can possibly remove the exceptions one-by-one.

 > > > - the other one appears to be inside the specific version of
 libsqlite on that old centos. I think I reported this as well last time,
 and more recent versions don't seem to have this issue. Anyway if we're
 not gonna upgrade, this is the repression I used on the centos buildbot:
 > >
 > > I'm not sure about how to comment on this.  Are you suggesting we
 > > upgrade sqlite3 on the buildbox that runs valgrind?
 >
 > actually i had my two systems mixed up; the buildbot version seems fine,
 it is my local centos VM that has the bad libsqlite :) (my original plan
 was to either upgrade or suppress that one issue)

 So I interpret it as we don't have to worry about this in the context
 of this branch.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2669#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development